Hi Simon,
On 09/15/14 15:57, Simon Glass wrote:
Add driver model support with this driver. In this case the platform data is in the driver. It would be better to put this into an SOC-specific file, but this is best attempted when more boards are moved over to use driver model.
Signed-off-by: Simon Glass sjg@chromium.org
drivers/gpio/mxc_gpio.c | 291 +++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 290 insertions(+), 1 deletion(-)
diff --git a/drivers/gpio/mxc_gpio.c b/drivers/gpio/mxc_gpio.c index 6a572d5..8669cf0 100644 --- a/drivers/gpio/mxc_gpio.c +++ b/drivers/gpio/mxc_gpio.c
[...]
+static int check_reserved(struct udevice *dev, unsigned offset,
const char *func)
Wouldn't "check_requested" be a better name here? And then also in the error message below?
+{
- struct mxc_bank_info *bank = dev_get_priv(dev);
- struct gpio_dev_priv *uc_priv = dev->uclass_priv;
- if (!*bank->label[offset]) {
Can we have here a more explicit (and descriptive) check for '\0'?
printf("mxc_gpio: %s: error: gpio %s%d not reserved\n",func, uc_priv->bank_name, offset);return -EPERM;- }
- return 0;
+}
[...]
+static int mxc_gpio_request(struct udevice *dev, unsigned offset,
const char *label)+{
- struct mxc_bank_info *bank = dev_get_priv(dev);
- if (*bank->label[offset])
Same here?
return -EBUSY;- strncpy(bank->label[offset], label, GPIO_NAME_SIZE);
- bank->label[offset][GPIO_NAME_SIZE - 1] = '\0';
- return 0;
+}
[...]
+static int mxc_gpio_get_function(struct udevice *dev, unsigned offset) +{
- struct mxc_bank_info *bank = dev_get_priv(dev);
- if (!*bank->label[offset])
and here.
return GPIOF_UNUSED;- /* GPIOF_FUNC is not implemented yet */
- if (mxc_gpio_is_output(bank->regs, offset))
return GPIOF_OUTPUT;- else
return GPIOF_INPUT;+}
[...]
+static int mxc_gpio_probe(struct udevice *dev) +{
- struct mxc_bank_info *bank = dev_get_priv(dev);
- struct mxc_gpio_plat *plat = dev_get_platdata(dev);
- struct gpio_dev_priv *uc_priv = dev->uclass_priv;
- int banknum;
- char *name = "";
I think you can skip this initialization, malloc will write into this pointer anyway.
- bank->regs = plat->regs;
- uc_priv->gpio_count = 32;
Isn't this GPIO_PER_BANK is defined for?
- name = malloc(8);
- if (!name)
return -ENOMEM;- banknum = plat - mxc_plat;
- if (banknum < 98)
sprintf(name, "GPIO%d_", banknum + 1);
The logic here (and the magic 98) is unclear. Can we have a bit more explanation here please?
- uc_priv->bank_name = name;
- return 0;
+}
[...]