[PATCH 2/4] ath79: add GPIO-latch driver for Mikrotik RB91xG

[Sergey Ryazanov]

On Thu, May 6, 2021 at 7:32 PM Denis Kalashnikov <denis281089 at gmail.com> wrote:

[skipped]

> +#define DRIVER_NAME  "rb91x-gpio-latch"

Maybe just "rb91x-gpio"? Looks like this board has no more crazy GPIO
controllers.

> +static int get(struct gpio_chip *gc, unsigned offset)
> +{
> +       return -ENOSYS;
> +}

[skipped]

> +static int direction_input(struct gpio_chip *gc, unsigned offset)
> +{
> +       return -ENOSYS;
> +}

Just drop these two callbacks and gpiolib will care about attempts to
toggle line direction to input on the output-only controller.

> +
> +static int direction_output(struct gpio_chip *gc, unsigned offset, int value)

Use driver name as function prefix.

> +{
> +       struct rb91x_ngl *ngl = gpiochip_get_data(gc);
> +
> +       ngl->latch_gpio_set_value(ngl, offset, value);
> +
> +       return 0;
> +}
> +
> +static int probe(struct platform_device *pdev)

ditto

> +{
> +       struct gpio_chip *gc;
> +       struct device_node *of_node = pdev->dev.of_node;
> +       struct device *dev = &pdev->dev, *parent = pdev->dev.parent;
> +       struct rb91x_ngl *ngl;
> +       int ret;
> +       u32 val;
> +
> +       pr_info(DRIVER_NAME " driver probe\n");

Remove this custom trace on final submission.

> +
> +       if (!parent)
> +               return -ENODEV;
> +
> +       gc = devm_kzalloc(&pdev->dev, sizeof(*gc), GFP_KERNEL);
> +       if (!gc)
> +               return -ENOMEM;
> +
> +       /*
> +       ret = of_property_read_u32(of_node, "base", &val);
> +       if (ret < 0) {
> +               dev_err(dev, "Could not read required 'base' property\n");
> +               return -EINVAL;
> +       }
> +       pr_info(DRIVER_NAME ": base = %d\n", val);
> +       gc->base = val;
> +       */
> +       gc->base = -1; /* Request dynamic allocation */
> +
> +       ngl = dev_get_drvdata(parent);
> +       if (!ngl) {
> +               pr_err(DRIVER_NAME " ngl is null\n");

dev_err(dev, ...)

> +               return -EINVAL;
> +       }
> +
> +       gc->label = DRIVER_NAME;
> +       gc->parent = dev;
> +       gc->can_sleep = true;
> +       gc->ngpio = ngl->latch_gpios_count(ngl);
> +       gc->get = get;
> +       gc->set = set;
> +       gc->direction_input = direction_input,
> +       gc->direction_output = direction_output;
> +       gc->of_node = of_node;
> +
> +       platform_set_drvdata(pdev, gc);
> +
> +       ret = gpiochip_add_data(gc, ngl);
> +       if (ret) {
> +               pr_err(DRIVER_NAME ": failed to add gpio chip: %d\n", ret);

dev_err(dev, ...)

> +               return ret;
> +       }
> +
> +       return 0;
> +}
> +
> +static int remove(struct platform_device *pdev)
> +{
> +       gpiochip_remove(platform_get_drvdata(pdev));
> +       return 0;
> +}
> +
> +static const struct of_device_id match[] = {

Use the driver name as prefix for global variables too.

> +       { .compatible = "mikrotik," DRIVER_NAME },
> +       {},
> +};

--
Sergey