Messages in this thread | ![/](/images/icornerl.gif) | | From | Linus Walleij <> | Date | Thu, 7 Jan 2021 11:47:40 +0100 | Subject | Re: [PATCH V4 5/5] gpio: gpio-xilinx: Add check if width exceeds 32 |
| |
On Thu, Jan 7, 2021 at 11:29 AM Michal Simek <michal.simek@xilinx.com> wrote: > On 07. 01. 21 11:17, Linus Walleij wrote: > > On Wed, Jan 6, 2021 at 1:27 PM Srinivas Neeli <srinivas.neeli@xilinx.com> wrote:
> >> @@ -591,6 +591,9 @@ static int xgpio_probe(struct platform_device *pdev) > >> if (of_property_read_u32(np, "xlnx,gpio-width", &chip->gpio_width[0])) > >> chip->gpio_width[0] = 32; > > > > This xlnx,gpio-width seems very much like the standard ngpios property > > from Documentation/devicetree/bindings/gpio/gpio.txt > > but I guess not much to do about that now. :/ > > > > Do you think you can add support for both? > > support for both is definitely possible but we need to handle also gpio > width for second channel referenced by xlnx,gpio2-widht now. > > It means we could end up in situation which can be misleading for users > where ngpios will be 10 and xlnx,gpio2-width another 10 and in total we > have 20 gpios.
OK that is confusing. Let's not do that then.
> I think that it is better not to start to mess with ngpios property not > to confuse people which are coming from other SOCs because ngpios can > suggest all gpios assigned to this controller.
OK I agree.
> >> + if (chip->gpio_width[0] > 32) > >> + return -EINVAL; > > > > This looks OK. > > Does it mean ack for this patch?
Yeah after explanations this patch is fine: Acked-by: Linus Walleij <linus.walleij@linaro.org>
It's just that this hardware with paired controllers is a bit weird so it will lead to discussions all the time because it's hard to understand.
Yours, Linus Walleij
| ![\](/images/icornerr.gif) |