linux-gpio.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Florian Fainelli <f.fainelli@gmail.com>
To: Stefan Wahren <stefan.wahren@i2se.com>, Lukas Wunner <lukas@wunner.de>
Cc: Linus Walleij <linus.walleij@linaro.org>,
	Nicolas Saenz Julienne <nsaenz@kernel.org>,
	Florian Fainelli <f.fainelli@gmail.com>,
	Ray Jui <rjui@broadcom.com>,
	Scott Branden <sbranden@broadcom.com>,
	bcm-kernel-feedback-list@broadcom.com,
	linux-gpio@vger.kernel.org,
	Lino Sanfilippo <LinoSanfilippo@gmx.de>,
	Philipp Rosenberger <p.rosenberger@kunbus.com>,
	linux-rpi-kernel@lists.infradead.org
Subject: Re: [PATCH] pinctrl: bcm2835: Use bcm2835 gpio_chip label for bcm2711
Date: Tue, 15 Feb 2022 09:17:49 -0800	[thread overview]
Message-ID: <2a5aad11-0ac6-943f-158a-585648b396be@gmail.com> (raw)
In-Reply-To: <09a5a501-40d7-4dae-56c9-a3796fc95c4b@i2se.com>

On 2/15/22 8:56 AM, Stefan Wahren wrote:
> Hi,
> 
> Am 15.02.22 um 15:44 schrieb Lukas Wunner:
> 
>> On Tue, Feb 15, 2022 at 01:00:47PM +0100, Stefan Wahren wrote:
>>> Am 15.02.22 um 06:52 schrieb Lukas Wunner:
>>>> Commit b1d84a3d0a26 ("pinctrl: bcm2835: Add support for all GPIOs on
>>>> BCM2711") used a different label for the bcm2711 gpio_chip vis-à-vis
>>>> the bcm2835.
>>>>
>>>> That breaks compatibility for GPIO_LOOKUP_IDX() and GPIO_HOG() clauses
>>>> when porting from older Raspberry Pi Compute Modules to the CM4 or CM4S.
>>> could you explain the breakage more in detail, is it kernel or user space?
>> This kernel module (which is sought to be upstreamed mid-term)
>> requests GPIOs at runtime for a chardev:
>>
>> https://github.com/RevolutionPi/piControl/blob/master/revpi_core.c#L50
>>
>> That fails on BCM2711 because a different label name was used,
>> even though the pin-controller is otherwise compatible to BCM2835.
> 
> sorry, but you cannot blame the mainline kernel for assumptions in out
> of tree drivers. What happens if another driver relies on the existing
> labeling?
> 
> How about detecting the platform via devicetree functions in your driver?
> 
>>
>>
>>> A little bit off topic, but what is this CM4S? Is it special version of
>>> the CM4? Can you provide a link or something?
>> BCM2711 in a CM1/CM3-compatible form factor.  There is no public
>> documentation at this point besides the device-tree overlay and
>> what's being discussed in the forums and on GitHub:
>>
>> https://github.com/raspberrypi/linux/blob/rpi-5.15.y/arch/arm/boot/dts/bcm2711-rpi-cm4s.dts
>> https://forums.raspberrypi.com/viewtopic.php?t=325975
>> https://github.com/search?q=cm4s&type=commits
> Thanks a lot.
>>
>>>> The name change seems unwarranted given it's essentially the same
>>>> hardware, so use the old name instead.
>>> I disagree at this point. The pinctrl of bcm2835 and bcm2711 are
>>> different. For example the bcm2835 has only 54 GPIOs while the bcm2711
>>> has 58.
>> Four additional GPIOs don't justify a different label name given the
>> pin-controller otherwise behaves the same.  We also had minimal
>> differences in pin assignment on BCM2835/6/7 and that didn't
>> justify a different label name either.
> 
> No, the GPIO pins of BCM2835/6/7 SoC are always identical from driver
> point of view, because they use the same devicetree compatible. It's the
> Raspberry Pi board which connect the GPIOs differently, that was one of
> the reasons for introduction of GPIO line names via devicetree.
> 
> I understand your pain, but i cannot give an Ack to this change.

I agree with Stefan here, besides changing the driver name now would
mean potentially breaking user-space since the driver name is visible in
a variety of places. Seems to me like this is too late, we should have
caught this during the introduction of 2711.
--
Florian

  reply	other threads:[~2022-02-15 17:17 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-02-15  5:52 [PATCH] pinctrl: bcm2835: Use bcm2835 gpio_chip label for bcm2711 Lukas Wunner
2022-02-15 12:00 ` Stefan Wahren
2022-02-15 14:44   ` Lukas Wunner
2022-02-15 16:56     ` Stefan Wahren
2022-02-15 17:17       ` Florian Fainelli [this message]
2022-02-16 10:05         ` Lukas Wunner

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=2a5aad11-0ac6-943f-158a-585648b396be@gmail.com \
    --to=f.fainelli@gmail.com \
    --cc=LinoSanfilippo@gmx.de \
    --cc=bcm-kernel-feedback-list@broadcom.com \
    --cc=linus.walleij@linaro.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=linux-rpi-kernel@lists.infradead.org \
    --cc=lukas@wunner.de \
    --cc=nsaenz@kernel.org \
    --cc=p.rosenberger@kunbus.com \
    --cc=rjui@broadcom.com \
    --cc=sbranden@broadcom.com \
    --cc=stefan.wahren@i2se.com \
    /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).