From: Andy Shevchenko <andy.shevchenko@gmail.com>
To: Aidan MacDonald <aidanmacdonald.0x0@gmail.com>
Cc: Linus Walleij <linus.walleij@linaro.org>,
Bartosz Golaszewski <brgl@bgdev.pl>,
Rob Herring <robh+dt@kernel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
Chen-Yu Tsai <wens@csie.org>, Jonathan Cameron <jic23@kernel.org>,
Lee Jones <lee.jones@linaro.org>,
Sebastian Reichel <sre@kernel.org>,
Mark Brown <broonie@kernel.org>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
Liam Girdwood <lgirdwood@gmail.com>,
Lars-Peter Clausen <lars@metafoo.de>,
"Rafael J. Wysocki" <rafael@kernel.org>,
"open list:GPIO SUBSYSTEM" <linux-gpio@vger.kernel.org>,
devicetree <devicetree@vger.kernel.org>,
linux-iio <linux-iio@vger.kernel.org>,
Linux PM <linux-pm@vger.kernel.org>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 10/10] pinctrl: Add AXP192 pin control driver
Date: Fri, 17 Jun 2022 18:08:09 +0200 [thread overview]
Message-ID: <CAHp75VfTLz7Zv1UPROuaGr8g88_BoDb2rd7GQ6m0dNEGSPSasQ@mail.gmail.com> (raw)
In-Reply-To: <VGkmH1cTj8QZxZXUjbuky58yY3X5QWSY@localhost>
On Fri, Jun 17, 2022 at 2:14 PM Aidan MacDonald
<aidanmacdonald.0x0@gmail.com> wrote:
> Andy Shevchenko <andy.shevchenko@gmail.com> writes:
> > On Fri, Jun 3, 2022 at 6:29 PM Aidan MacDonald
> > <aidanmacdonald.0x0@gmail.com> wrote:
...
> >> + if ((val & reginfo->mask) == (input_muxvals[offset] << (ffs(reginfo->mask) - 1)))
> >> + return GPIO_LINE_DIRECTION_IN;
> >
> >> + else
> >
> > Redundant.
> > Also applies for the other similar cases in your code. Note, this is
> > also redundant for 'continue' and 'break' in case of loops.
>
> Sorry, I'm not sure what you're referring to here. The "else"?
Yes.
> I'm missing the generalization.
>
> >> + return GPIO_LINE_DIRECTION_OUT;
...
> >> + pctl->chip.to_irq = axp192_gpio_to_irq;
> >
> > Why a custom method?
>
> The irq chip is part of the mfd device, not the gpio chip. There does
> not seem to be any default implementation for this case so I have to
> provide one. A similar example is gpio-wm8994.
>
> I did notice I'm doing something wrong by calling regmap_irq_get_virq()
> in the probe function, which creates an irq mapping; I think I should be
> doing that in the to_irq() callback like the other drivers do.
It may be done using different approaches, but this part should be
carefully reviewed by GPIO / pin control maintainers.
...
> Ah, sorry, I see that function is deprecated. The documentation points
> to doing this in the device tree instead. So if I understand correctly
> I should follow the example of pinctrl-thunderbay and add gpio-ranges:
>
> pinctrl0: gpio@0 {
> compatible = "x-powers,axp192-gpio";
> gpio-controller;
> #gpio-cells = <2>;
> gpio-ranges = <&pinctrl0 0 0 6>;
> };
>
> which means I'll have to update the gpio DT bindings. I'm guessing the
> callback you mentioned is add_pin_ranges() or of_gpio_ranges_fallback()
> but neither of those seem appropriate in this case. The DT node should
> be good enough.
Sounds good.
--
With Best Regards,
Andy Shevchenko
prev parent reply other threads:[~2022-06-17 16:08 UTC|newest]
Thread overview: 36+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-06-03 13:57 [PATCH 00/10] Add support for AXP192 PMIC Aidan MacDonald
2022-06-03 13:57 ` [PATCH 01/10] regmap-irq: Add get_irq_reg to support unusual register layouts Aidan MacDonald
2022-06-06 17:43 ` Guru Das Srinagesh
2022-06-07 10:46 ` Aidan MacDonald
2022-06-03 13:57 ` [PATCH 02/10] dt-bindings: mfd: add bindings for AXP192 MFD device Aidan MacDonald
2022-06-05 22:49 ` Rob Herring
2022-06-27 11:47 ` Lee Jones
2022-06-03 13:57 ` [PATCH 03/10] dt-bindings: iio: adc: axp209: Add AXP192 compatible Aidan MacDonald
2022-06-03 16:34 ` Jonathan Cameron
2022-06-04 11:33 ` Aidan MacDonald
2022-06-05 22:50 ` Rob Herring
2022-06-03 13:57 ` [PATCH 04/10] dt-bindings: power: supply: axp20x: " Aidan MacDonald
2022-06-05 22:50 ` Rob Herring
2022-06-03 13:57 ` [PATCH 05/10] dt-bindings: gpio: Add AXP192 GPIO bindings Aidan MacDonald
2022-06-05 22:55 ` Rob Herring
2022-06-07 10:34 ` Aidan MacDonald
2022-06-07 15:17 ` Rob Herring
2022-06-07 15:40 ` Aidan MacDonald
2022-06-03 13:57 ` [PATCH 06/10] mfd: axp20x: Add support for AXP192 Aidan MacDonald
2022-06-27 11:54 ` Lee Jones
2022-06-27 13:02 ` Aidan MacDonald
2022-06-03 13:57 ` [PATCH 07/10] regulator: " Aidan MacDonald
2022-06-06 14:36 ` Mark Brown
2022-06-03 13:57 ` [PATCH 08/10] iio: adc: axp20x_adc: " Aidan MacDonald
2022-06-03 16:47 ` Jonathan Cameron
2022-06-04 11:47 ` Aidan MacDonald
2022-06-04 14:27 ` Jonathan Cameron
2022-06-07 10:49 ` Aidan MacDonald
2022-06-03 13:57 ` [PATCH 09/10] power: supply: axp20x_usb_power: " Aidan MacDonald
2022-06-05 15:13 ` kernel test robot
2022-06-03 13:57 ` [PATCH 10/10] pinctrl: Add AXP192 pin control driver Aidan MacDonald
2022-06-15 13:44 ` Linus Walleij
2022-06-15 18:06 ` Michael Walle
2022-06-15 14:51 ` Andy Shevchenko
2022-06-17 12:15 ` Aidan MacDonald
2022-06-17 16:08 ` Andy Shevchenko [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAHp75VfTLz7Zv1UPROuaGr8g88_BoDb2rd7GQ6m0dNEGSPSasQ@mail.gmail.com \
--to=andy.shevchenko@gmail.com \
--cc=aidanmacdonald.0x0@gmail.com \
--cc=brgl@bgdev.pl \
--cc=broonie@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=gregkh@linuxfoundation.org \
--cc=jic23@kernel.org \
--cc=krzysztof.kozlowski+dt@linaro.org \
--cc=lars@metafoo.de \
--cc=lee.jones@linaro.org \
--cc=lgirdwood@gmail.com \
--cc=linus.walleij@linaro.org \
--cc=linux-gpio@vger.kernel.org \
--cc=linux-iio@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=rafael@kernel.org \
--cc=robh+dt@kernel.org \
--cc=sre@kernel.org \
--cc=wens@csie.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).