On Mon, Jan 22, 2018 at 7:04 AM, Jonathan Neuschäfer
<j.neuschaefer@xxxxxxx> wrote:
Style issues below.
> +#define HW_GPIO_OWNER 0x3c
> +
> +
> +struct hlwd_gpio {
No need extra empty line in between.
> + struct gpio_chip gpioc;
> + void __iomem *regs;
> + struct device *dev;
> +};
> +
> +static int hlwd_gpio_probe(struct platform_device *pdev)
> +{
> + struct hlwd_gpio *hlwd;
> + struct resource *regs_resource;
> + u32 ngpios;
> + int res;
> +
> + hlwd = devm_kzalloc(&pdev->dev, sizeof(*hlwd), GFP_KERNEL);
> + if (!hlwd)
> + return -ENOMEM;
> +
> + /* Save the struct device pointer so dev_info, etc. can be used. */
Useless.
> + hlwd->dev = &pdev->dev;
> +
> + regs_resource = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + if (IS_ERR(regs_resource))
> + return PTR_ERR(regs_resource);
> +
This is redundant. Below does it for ya.
> + hlwd->regs = devm_ioremap_resource(&pdev->dev, regs_resource);
> + if (IS_ERR(hlwd->regs))
> + return PTR_ERR(hlwd->regs);
> + res = bgpio_init(&hlwd->gpioc, &pdev->dev, 4,
> + hlwd->regs + HW_GPIOB_IN, hlwd->regs + HW_GPIOB_OUT,
> + NULL, hlwd->regs + HW_GPIOB_DIR, NULL,
> + BGPIOF_BIG_ENDIAN_BYTE_ORDER);
> +
Remove this extra line.
> + if (res < 0) {
> + dev_warn(hlwd->dev, "bgpio_init failed: %d\n", res);
> + return res;
> + }
> + if (of_property_read_u32(pdev->dev.of_node, "ngpios", &ngpios))
> + ngpios = 32;
A nit: I would rather go with
res = of_property_read(...);
if (res)
ngpios = 32;
--
With Best Regards,
Andy Shevchenko
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
