Re: [PATCH v6 081/102] x86: Add a generic Intel GPIO driver
Hi Wolfgang,
On Fri, 13 Dec 2019 at 01:49, Wolfgang Wallner
wrote:
>
> Hi Simon, Bin,
>
> > +static int intel_gpio_direction_output(struct udevice *dev, uint offset,
> > +int value)
> > +{
> > + struct udevice *pinctrl = dev_get_parent(dev);
> > + uint config_offset = intel_pinctrl_get_config_reg_addr(pinctrl,
> > offset);
> > +
> > + pcr_clrsetbits32(dev, config_offset,
>
> I think we should pass 'pinctrl' instead of 'dev' here.
> As far as I understand the code the function pcr_clrsetbits32 expects a
> pinctrl
> device with a p2sb parent.
>
> > + PAD_CFG0_MODE_MASK | PAD_CFG0_RX_STATE |
> > + PAD_CFG0_TX_DISABLE,
>
> We also need to clear the bit PAD_CFG0_TX_STATE here.
> Otherwise if a gpio is set to high once it can never be set to low again.
>
> > + PAD_CFG0_MODE_GPIO | PAD_CFG0_RX_DISABLE |
> > + (value ? PAD_CFG0_TX_STATE : 0));
> > +
I see you have sent patches for these, thank you,
Regards,
Simon
Re: [PATCH v6 081/102] x86: Add a generic Intel GPIO driver
Hi Simon, Bin,
> +static int intel_gpio_direction_output(struct udevice *dev, uint offset,
> +int value)
> +{
> + struct udevice *pinctrl = dev_get_parent(dev);
> + uint config_offset = intel_pinctrl_get_config_reg_addr(pinctrl, offset);
> +
> + pcr_clrsetbits32(dev, config_offset,
I think we should pass 'pinctrl' instead of 'dev' here.
As far as I understand the code the function pcr_clrsetbits32 expects a pinctrl
device with a p2sb parent.
> + PAD_CFG0_MODE_MASK | PAD_CFG0_RX_STATE |
> + PAD_CFG0_TX_DISABLE,
We also need to clear the bit PAD_CFG0_TX_STATE here.
Otherwise if a gpio is set to high once it can never be set to low again.
> + PAD_CFG0_MODE_GPIO | PAD_CFG0_RX_DISABLE |
> + (value ? PAD_CFG0_TX_STATE : 0));
> +
> + return 0;
> +}
regards, Wolfgang
Re: [PATCH v6 081/102] x86: Add a generic Intel GPIO driver
On Sun, Dec 8, 2019 at 4:01 PM Bin Meng wrote: > > On Sat, Dec 7, 2019 at 12:54 PM Simon Glass wrote: > > > > Add a GPIO driver which uses the pinctrl driver to access the pad > > information. This driver relies on the GPIO nodes being subnodes to the > > pinctrl device. > > > > Signed-off-by: Simon Glass > > --- > > > > Changes in v6: > > - Fix 'hone' typo > > - Remove the * in the first line of the binding file > > - Use 'north' as the node name instead of 'n' > > - Use a generic compatible string intel,gpio > > > > Changes in v5: None > > Changes in v4: None > > Changes in v3: None > > Changes in v2: None > > > > .../gpio/intel,apl-gpio.txt | 55 ++ > > drivers/gpio/Kconfig | 9 + > > drivers/gpio/Makefile | 1 + > > drivers/gpio/intel_gpio.c | 161 ++ > > 4 files changed, 226 insertions(+) > > create mode 100644 doc/device-tree-bindings/gpio/intel,apl-gpio.txt > > create mode 100644 drivers/gpio/intel_gpio.c > > > > Reviewed-by: Bin Meng applied to u-boot-x86/next, thanks!
Re: [PATCH v6 081/102] x86: Add a generic Intel GPIO driver
On Sat, Dec 7, 2019 at 12:54 PM Simon Glass wrote: > > Add a GPIO driver which uses the pinctrl driver to access the pad > information. This driver relies on the GPIO nodes being subnodes to the > pinctrl device. > > Signed-off-by: Simon Glass > --- > > Changes in v6: > - Fix 'hone' typo > - Remove the * in the first line of the binding file > - Use 'north' as the node name instead of 'n' > - Use a generic compatible string intel,gpio > > Changes in v5: None > Changes in v4: None > Changes in v3: None > Changes in v2: None > > .../gpio/intel,apl-gpio.txt | 55 ++ > drivers/gpio/Kconfig | 9 + > drivers/gpio/Makefile | 1 + > drivers/gpio/intel_gpio.c | 161 ++ > 4 files changed, 226 insertions(+) > create mode 100644 doc/device-tree-bindings/gpio/intel,apl-gpio.txt > create mode 100644 drivers/gpio/intel_gpio.c > Reviewed-by: Bin Meng
