On Mon, Jan 22, 2018 at 7:04 AM, Jonathan Neusch=C3=A4fer
[off-list ref] 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 =3D 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 =3D &pdev->dev;
+

+       regs_resource =3D 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 =3D devm_ioremap_resource(&pdev->dev, regs_resource);
+       if (IS_ERR(hlwd->regs))
+               return PTR_ERR(hlwd->regs);

+       res =3D bgpio_init(&hlwd->gpioc, &pdev->dev, 4,
+                       hlwd->regs + HW_GPIOB_IN, hlwd->regs + HW_GPIOB_O=

UT,

+                       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 =3D 32;

A nit: I would rather go with
res =3D of_property_read(...);
if (res)
  ngpios =3D 32;

--=20
With Best Regards,
Andy Shevchenko