[PATCH v1] serdev: Set fwnode for serdev devices

[PATCH v1] serdev: Set fwnode for serdev devices

[Saravana Kannan]

This allow fw_devlink to do dependency tracking for serdev devices.

Reported-by: Florian Fainelli <f.fainelli@gmail.com>
Link: https://lore.kernel.org/lkml/03b70a8a-0591-f28b-a567-9d2f736f17e5@gmail.com/
Cc: Stefan Wahren <stefan.wahren@i2se.com>
Signed-off-by: Saravana Kannan <saravanak@google.com>
---
Florian,

Can you give it a shot and a tested-by please?

-Saravana

 drivers/tty/serdev/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c
index aa80de3a8194..678014253b7b 100644
--- a/drivers/tty/serdev/core.c
+++ b/drivers/tty/serdev/core.c
@@ -534,7 +534,7 @@ static int of_serdev_register_devices(struct serdev_controller *ctrl)
 		if (!serdev)
 			continue;
 
-		serdev->dev.of_node = node;
+		device_set_node(&serdev->dev, of_fwnode_handle(node));
 
 		err = serdev_device_add(serdev);
 		if (err) {
-- 
2.39.2.722.g9855ee24e9-goog

Re: [PATCH v1] serdev: Set fwnode for serdev devices

[Florian Fainelli]

On 3/1/2023 6:35 PM, Saravana Kannan wrote:
> This allow fw_devlink to do dependency tracking for serdev devices.
> 
> Reported-by: Florian Fainelli <f.fainelli@gmail.com>
> Link: https://lore.kernel.org/lkml/03b70a8a-0591-f28b-a567-9d2f736f17e5@gmail.com/
> Cc: Stefan Wahren <stefan.wahren@i2se.com>
> Signed-off-by: Saravana Kannan <saravanak@google.com>

Tested-by: Florian Fainelli <f.fainelli@gmail.com>

Thanks for the quick fix!
-- 
Florian

Re: [PATCH v1] serdev: Set fwnode for serdev devices

[Stefan Wahren]

Hi Saravana,

Am 02.03.23 um 03:35 schrieb Saravana Kannan:
> This allow fw_devlink to do dependency tracking for serdev devices.
>
> Reported-by: Florian Fainelli <f.fainelli@gmail.com>
> Link: https://lore.kernel.org/lkml/03b70a8a-0591-f28b-a567-9d2f736f17e5@gmail.com/
> Cc: Stefan Wahren <stefan.wahren@i2se.com>
> Signed-off-by: Saravana Kannan <saravanak@google.com>

since this fixes an issue on Raspberry Pi 4, shouldn't this be mentioned 
in the commit message and providing a Fixes tag?

Thanks
Stefan

> ---
> Florian,
>
> Can you give it a shot and a tested-by please?
>
> -Saravana
>
>   drivers/tty/serdev/core.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c
> index aa80de3a8194..678014253b7b 100644
> --- a/drivers/tty/serdev/core.c
> +++ b/drivers/tty/serdev/core.c
> @@ -534,7 +534,7 @@ static int of_serdev_register_devices(struct serdev_controller *ctrl)
>   		if (!serdev)
>   			continue;
>   
> -		serdev->dev.of_node = node;
> +		device_set_node(&serdev->dev, of_fwnode_handle(node));
>   
>   		err = serdev_device_add(serdev);
>   		if (err) {

Re: [PATCH v1] serdev: Set fwnode for serdev devices

[Saravana Kannan]

On Thu, Mar 2, 2023 at 9:01 AM Stefan Wahren <stefan.wahren@i2se.com> wrote:
>
> Hi Saravana,
>
> Am 02.03.23 um 03:35 schrieb Saravana Kannan:
> > This allow fw_devlink to do dependency tracking for serdev devices.
> >
> > Reported-by: Florian Fainelli <f.fainelli@gmail.com>
> > Link: https://lore.kernel.org/lkml/03b70a8a-0591-f28b-a567-9d2f736f17e5@gmail.com/
> > Cc: Stefan Wahren <stefan.wahren@i2se.com>
> > Signed-off-by: Saravana Kannan <saravanak@google.com>
>
> since this fixes an issue on Raspberry Pi 4, shouldn't this be mentioned
> in the commit message and providing a Fixes tag?

So RPi 4 was never creating a device links between serdev devices and
their consumers. The error message was just a new one I added and we
are noticing and catching the fact that serdev wasn't setting fwnode
for a device.

I'm also not sure if I can say this commit "Fixes" an issue in serdev
core because when serdev core was written, fw_devlink wasn't a thing.
Once I add Fixes, people will start pulling this into stable
branches/other trees where I don't think this should be pulled into
older stable branches.

-Saravana

>
> Thanks
> Stefan
>
> > ---
> > Florian,
> >
> > Can you give it a shot and a tested-by please?
> >
> > -Saravana
> >
> >   drivers/tty/serdev/core.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c
> > index aa80de3a8194..678014253b7b 100644
> > --- a/drivers/tty/serdev/core.c
> > +++ b/drivers/tty/serdev/core.c
> > @@ -534,7 +534,7 @@ static int of_serdev_register_devices(struct serdev_controller *ctrl)
> >               if (!serdev)
> >                       continue;
> >
> > -             serdev->dev.of_node = node;
> > +             device_set_node(&serdev->dev, of_fwnode_handle(node));
> >
> >               err = serdev_device_add(serdev);
> >               if (err) {

Re: [PATCH v1] serdev: Set fwnode for serdev devices

[Florian Fainelli]

On 3/2/2023 9:20 AM, Saravana Kannan wrote:
> On Thu, Mar 2, 2023 at 9:01 AM Stefan Wahren <stefan.wahren@i2se.com> wrote:
>>
>> Hi Saravana,
>>
>> Am 02.03.23 um 03:35 schrieb Saravana Kannan:
>>> This allow fw_devlink to do dependency tracking for serdev devices.
>>>
>>> Reported-by: Florian Fainelli <f.fainelli@gmail.com>
>>> Link: https://lore.kernel.org/lkml/03b70a8a-0591-f28b-a567-9d2f736f17e5@gmail.com/
>>> Cc: Stefan Wahren <stefan.wahren@i2se.com>
>>> Signed-off-by: Saravana Kannan <saravanak@google.com>
>>
>> since this fixes an issue on Raspberry Pi 4, shouldn't this be mentioned
>> in the commit message and providing a Fixes tag?
> 
> So RPi 4 was never creating a device links between serdev devices and
> their consumers. The error message was just a new one I added and we
> are noticing and catching the fact that serdev wasn't setting fwnode
> for a device.
> 
> I'm also not sure if I can say this commit "Fixes" an issue in serdev
> core because when serdev core was written, fw_devlink wasn't a thing.
> Once I add Fixes, people will start pulling this into stable
> branches/other trees where I don't think this should be pulled into
> older stable branches.

That is kind of the point of Fixes: tag, is not it? It is appropriate to 
list a commit that is not specific to serdev, but maybe a particular 
point into the fw_devlink history. Given this did not appear to have a 
functional impact, we could go without one.
-- 
Florian

Re: [PATCH v1] serdev: Set fwnode for serdev devices

[Saravana Kannan]

On Thu, Mar 2, 2023 at 9:51 AM Florian Fainelli <f.fainelli@gmail.com> wrote:
>
>
>
> On 3/2/2023 9:20 AM, Saravana Kannan wrote:
> > On Thu, Mar 2, 2023 at 9:01 AM Stefan Wahren <stefan.wahren@i2se.com> wrote:
> >>
> >> Hi Saravana,
> >>
> >> Am 02.03.23 um 03:35 schrieb Saravana Kannan:
> >>> This allow fw_devlink to do dependency tracking for serdev devices.
> >>>
> >>> Reported-by: Florian Fainelli <f.fainelli@gmail.com>
> >>> Link: https://lore.kernel.org/lkml/03b70a8a-0591-f28b-a567-9d2f736f17e5@gmail.com/
> >>> Cc: Stefan Wahren <stefan.wahren@i2se.com>
> >>> Signed-off-by: Saravana Kannan <saravanak@google.com>
> >>
> >> since this fixes an issue on Raspberry Pi 4, shouldn't this be mentioned
> >> in the commit message and providing a Fixes tag?
> >
> > So RPi 4 was never creating a device links between serdev devices and
> > their consumers. The error message was just a new one I added and we
> > are noticing and catching the fact that serdev wasn't setting fwnode
> > for a device.
> >
> > I'm also not sure if I can say this commit "Fixes" an issue in serdev
> > core because when serdev core was written, fw_devlink wasn't a thing.
> > Once I add Fixes, people will start pulling this into stable
> > branches/other trees where I don't think this should be pulled into
> > older stable branches.
>
> That is kind of the point of Fixes: tag, is not it? It is appropriate to
> list a commit that is not specific to serdev, but maybe a particular
> point into the fw_devlink history.

I don't want to pick an arbitrary point in fw_devlink as I don't want
people picking this up with some old version of fw_devlink and having
to support it there.

> Given this did not appear to have a
> functional impact, we could go without one.

This is my take too.

Greg/Rob,

If you really want a Fixes here, can you please just add it instead of
a v2 patch just for that? You can use this commit:
3fb16866b51d driver core: fw_devlink: Make cycle detection more robust

-Saravana

Re: [PATCH v1] serdev: Set fwnode for serdev devices

[Saravana Kannan]

On Thu, Mar 2, 2023 at 10:07 AM Saravana Kannan <saravanak@google.com> wrote:
>
> On Thu, Mar 2, 2023 at 9:51 AM Florian Fainelli <f.fainelli@gmail.com> wrote:
> >
> >
> >
> > On 3/2/2023 9:20 AM, Saravana Kannan wrote:
> > > On Thu, Mar 2, 2023 at 9:01 AM Stefan Wahren <stefan.wahren@i2se.com> wrote:
> > >>
> > >> Hi Saravana,
> > >>
> > >> Am 02.03.23 um 03:35 schrieb Saravana Kannan:
> > >>> This allow fw_devlink to do dependency tracking for serdev devices.
> > >>>
> > >>> Reported-by: Florian Fainelli <f.fainelli@gmail.com>
> > >>> Link: https://lore.kernel.org/lkml/03b70a8a-0591-f28b-a567-9d2f736f17e5@gmail.com/
> > >>> Cc: Stefan Wahren <stefan.wahren@i2se.com>
> > >>> Signed-off-by: Saravana Kannan <saravanak@google.com>
> > >>
> > >> since this fixes an issue on Raspberry Pi 4, shouldn't this be mentioned
> > >> in the commit message and providing a Fixes tag?
> > >
> > > So RPi 4 was never creating a device links between serdev devices and
> > > their consumers. The error message was just a new one I added and we
> > > are noticing and catching the fact that serdev wasn't setting fwnode
> > > for a device.
> > >
> > > I'm also not sure if I can say this commit "Fixes" an issue in serdev
> > > core because when serdev core was written, fw_devlink wasn't a thing.
> > > Once I add Fixes, people will start pulling this into stable
> > > branches/other trees where I don't think this should be pulled into
> > > older stable branches.
> >
> > That is kind of the point of Fixes: tag, is not it? It is appropriate to
> > list a commit that is not specific to serdev, but maybe a particular
> > point into the fw_devlink history.
>
> I don't want to pick an arbitrary point in fw_devlink as I don't want
> people picking this up with some old version of fw_devlink and having
> to support it there.
>
> > Given this did not appear to have a
> > functional impact, we could go without one.
>
> This is my take too.
>
> Greg/Rob,
>
> If you really want a Fixes here, can you please just add it instead of
> a v2 patch just for that? You can use this commit:
> 3fb16866b51d driver core: fw_devlink: Make cycle detection more robust

Rob/Greg,

Can you pick this up for 6.3-rc2 please?

-Saravana

Re: [PATCH v1] serdev: Set fwnode for serdev devices

[Greg Kroah-Hartman]

On Mon, Mar 06, 2023 at 08:47:48PM -0800, Saravana Kannan wrote:
> On Thu, Mar 2, 2023 at 10:07 AM Saravana Kannan <saravanak@google.com> wrote:
> >
> > On Thu, Mar 2, 2023 at 9:51 AM Florian Fainelli <f.fainelli@gmail.com> wrote:
> > >
> > >
> > >
> > > On 3/2/2023 9:20 AM, Saravana Kannan wrote:
> > > > On Thu, Mar 2, 2023 at 9:01 AM Stefan Wahren <stefan.wahren@i2se.com> wrote:
> > > >>
> > > >> Hi Saravana,
> > > >>
> > > >> Am 02.03.23 um 03:35 schrieb Saravana Kannan:
> > > >>> This allow fw_devlink to do dependency tracking for serdev devices.
> > > >>>
> > > >>> Reported-by: Florian Fainelli <f.fainelli@gmail.com>
> > > >>> Link: https://lore.kernel.org/lkml/03b70a8a-0591-f28b-a567-9d2f736f17e5@gmail.com/
> > > >>> Cc: Stefan Wahren <stefan.wahren@i2se.com>
> > > >>> Signed-off-by: Saravana Kannan <saravanak@google.com>
> > > >>
> > > >> since this fixes an issue on Raspberry Pi 4, shouldn't this be mentioned
> > > >> in the commit message and providing a Fixes tag?
> > > >
> > > > So RPi 4 was never creating a device links between serdev devices and
> > > > their consumers. The error message was just a new one I added and we
> > > > are noticing and catching the fact that serdev wasn't setting fwnode
> > > > for a device.
> > > >
> > > > I'm also not sure if I can say this commit "Fixes" an issue in serdev
> > > > core because when serdev core was written, fw_devlink wasn't a thing.
> > > > Once I add Fixes, people will start pulling this into stable
> > > > branches/other trees where I don't think this should be pulled into
> > > > older stable branches.
> > >
> > > That is kind of the point of Fixes: tag, is not it? It is appropriate to
> > > list a commit that is not specific to serdev, but maybe a particular
> > > point into the fw_devlink history.
> >
> > I don't want to pick an arbitrary point in fw_devlink as I don't want
> > people picking this up with some old version of fw_devlink and having
> > to support it there.
> >
> > > Given this did not appear to have a
> > > functional impact, we could go without one.
> >
> > This is my take too.
> >
> > Greg/Rob,
> >
> > If you really want a Fixes here, can you please just add it instead of
> > a v2 patch just for that? You can use this commit:
> > 3fb16866b51d driver core: fw_devlink: Make cycle detection more robust
> 
> Rob/Greg,
> 
> Can you pick this up for 6.3-rc2 please?

Will do, my queue is huge at the moment, it might be -rc3...

Re: [PATCH v1] serdev: Set fwnode for serdev devices

[Stefan Wahren]

Hi,

Am 02.03.23 um 18:51 schrieb Florian Fainelli:
>
>
> On 3/2/2023 9:20 AM, Saravana Kannan wrote:
>> On Thu, Mar 2, 2023 at 9:01 AM Stefan Wahren <stefan.wahren@i2se.com> 
>> wrote:
>>>
>>> Hi Saravana,
>>>
>>> Am 02.03.23 um 03:35 schrieb Saravana Kannan:
>>>> This allow fw_devlink to do dependency tracking for serdev devices.
>>>>
>>>> Reported-by: Florian Fainelli <f.fainelli@gmail.com>
>>>> Link: 
>>>> https://lore.kernel.org/lkml/03b70a8a-0591-f28b-a567-9d2f736f17e5@gmail.com/
>>>> Cc: Stefan Wahren <stefan.wahren@i2se.com>
>>>> Signed-off-by: Saravana Kannan <saravanak@google.com>
>>>
>>> since this fixes an issue on Raspberry Pi 4, shouldn't this be 
>>> mentioned
>>> in the commit message and providing a Fixes tag?
>>
>> So RPi 4 was never creating a device links between serdev devices and
>> their consumers. The error message was just a new one I added and we
>> are noticing and catching the fact that serdev wasn't setting fwnode
>> for a device.
>>
>> I'm also not sure if I can say this commit "Fixes" an issue in serdev
>> core because when serdev core was written, fw_devlink wasn't a thing.
>> Once I add Fixes, people will start pulling this into stable
>> branches/other trees where I don't think this should be pulled into
>> older stable branches.
>
> That is kind of the point of Fixes: tag, is not it? It is appropriate 
> to list a commit that is not specific to serdev, but maybe a 
> particular point into the fw_devlink history. Given this did not 
> appear to have a functional impact, we could go without one.

i was under the impression that this issue breaks at least Bluetooth on 
Raspberry Pi 4 because the driver is never probed. I cannot see the 
success output in Florian's trace. Something like this:

[    7.124879] hci_uart_bcm serial0-0: supply vbat not found, using 
dummy regulator
[    7.131743] hci_uart_bcm serial0-0: supply vddio not found, using 
dummy regulator
...
[    7.517249] Bluetooth: hci0: BCM: chip id 107
[    7.517499] Bluetooth: hci0: BCM: features 0x2f
[    7.519757] Bluetooth: hci0: BCM4345C0
[    7.519768] Bluetooth: hci0: BCM4345C0 (003.001.025) build 0000
[    7.539495] Bluetooth: hci0: BCM4345C0 'brcm/BCM4345C0.hcd' Patch
...
[    8.348831] Bluetooth: hci0: BCM43455 37.4MHz Raspberry Pi 3+
[    8.348845] Bluetooth: hci0: BCM4345C0 (003.001.025) build 0342

I just want to make sure that 6.2 doesn't have a regression.

Re: [PATCH v1] serdev: Set fwnode for serdev devices

[Florian Fainelli]

On 3/3/23 03:57, Stefan Wahren wrote:
> Hi,
> 
> Am 02.03.23 um 18:51 schrieb Florian Fainelli:
>>
>>
>> On 3/2/2023 9:20 AM, Saravana Kannan wrote:
>>> On Thu, Mar 2, 2023 at 9:01 AM Stefan Wahren <stefan.wahren@i2se.com> 
>>> wrote:
>>>>
>>>> Hi Saravana,
>>>>
>>>> Am 02.03.23 um 03:35 schrieb Saravana Kannan:
>>>>> This allow fw_devlink to do dependency tracking for serdev devices.
>>>>>
>>>>> Reported-by: Florian Fainelli <f.fainelli@gmail.com>
>>>>> Link: 
>>>>> https://lore.kernel.org/lkml/03b70a8a-0591-f28b-a567-9d2f736f17e5@gmail.com/
>>>>> Cc: Stefan Wahren <stefan.wahren@i2se.com>
>>>>> Signed-off-by: Saravana Kannan <saravanak@google.com>
>>>>
>>>> since this fixes an issue on Raspberry Pi 4, shouldn't this be 
>>>> mentioned
>>>> in the commit message and providing a Fixes tag?
>>>
>>> So RPi 4 was never creating a device links between serdev devices and
>>> their consumers. The error message was just a new one I added and we
>>> are noticing and catching the fact that serdev wasn't setting fwnode
>>> for a device.
>>>
>>> I'm also not sure if I can say this commit "Fixes" an issue in serdev
>>> core because when serdev core was written, fw_devlink wasn't a thing.
>>> Once I add Fixes, people will start pulling this into stable
>>> branches/other trees where I don't think this should be pulled into
>>> older stable branches.
>>
>> That is kind of the point of Fixes: tag, is not it? It is appropriate 
>> to list a commit that is not specific to serdev, but maybe a 
>> particular point into the fw_devlink history. Given this did not 
>> appear to have a functional impact, we could go without one.
> 
> i was under the impression that this issue breaks at least Bluetooth on 
> Raspberry Pi 4 because the driver is never probed. I cannot see the 
> success output in Florian's trace. Something like this:
> 
> [    7.124879] hci_uart_bcm serial0-0: supply vbat not found, using 
> dummy regulator
> [    7.131743] hci_uart_bcm serial0-0: supply vddio not found, using 
> dummy regulator
> ...
> [    7.517249] Bluetooth: hci0: BCM: chip id 107
> [    7.517499] Bluetooth: hci0: BCM: features 0x2f
> [    7.519757] Bluetooth: hci0: BCM4345C0
> [    7.519768] Bluetooth: hci0: BCM4345C0 (003.001.025) build 0000
> [    7.539495] Bluetooth: hci0: BCM4345C0 'brcm/BCM4345C0.hcd' Patch
> ...
> [    8.348831] Bluetooth: hci0: BCM43455 37.4MHz Raspberry Pi 3+
> [    8.348845] Bluetooth: hci0: BCM4345C0 (003.001.025) build 0342
> 
> I just want to make sure that 6.2 doesn't have a regression.

My configuration uses hci_uart as a module, and it would always load 
fine, but I suppose I can make sure that even built-in this works 
properly. Give me a day or two to test that.
-- 
Florian

Re: [PATCH v1] serdev: Set fwnode for serdev devices

[Saravana Kannan]

On Fri, Mar 3, 2023 at 9:22 AM Florian Fainelli <f.fainelli@gmail.com> wrote:
>
> On 3/3/23 03:57, Stefan Wahren wrote:
> > Hi,
> >
> > Am 02.03.23 um 18:51 schrieb Florian Fainelli:
> >>
> >>
> >> On 3/2/2023 9:20 AM, Saravana Kannan wrote:
> >>> On Thu, Mar 2, 2023 at 9:01 AM Stefan Wahren <stefan.wahren@i2se.com>
> >>> wrote:
> >>>>
> >>>> Hi Saravana,
> >>>>
> >>>> Am 02.03.23 um 03:35 schrieb Saravana Kannan:
> >>>>> This allow fw_devlink to do dependency tracking for serdev devices.
> >>>>>
> >>>>> Reported-by: Florian Fainelli <f.fainelli@gmail.com>
> >>>>> Link:
> >>>>> https://lore.kernel.org/lkml/03b70a8a-0591-f28b-a567-9d2f736f17e5@gmail.com/
> >>>>> Cc: Stefan Wahren <stefan.wahren@i2se.com>
> >>>>> Signed-off-by: Saravana Kannan <saravanak@google.com>
> >>>>
> >>>> since this fixes an issue on Raspberry Pi 4, shouldn't this be
> >>>> mentioned
> >>>> in the commit message and providing a Fixes tag?
> >>>
> >>> So RPi 4 was never creating a device links between serdev devices and
> >>> their consumers. The error message was just a new one I added and we
> >>> are noticing and catching the fact that serdev wasn't setting fwnode
> >>> for a device.
> >>>
> >>> I'm also not sure if I can say this commit "Fixes" an issue in serdev
> >>> core because when serdev core was written, fw_devlink wasn't a thing.
> >>> Once I add Fixes, people will start pulling this into stable
> >>> branches/other trees where I don't think this should be pulled into
> >>> older stable branches.
> >>
> >> That is kind of the point of Fixes: tag, is not it? It is appropriate
> >> to list a commit that is not specific to serdev, but maybe a
> >> particular point into the fw_devlink history. Given this did not
> >> appear to have a functional impact, we could go without one.
> >
> > i was under the impression that this issue breaks at least Bluetooth on
> > Raspberry Pi 4 because the driver is never probed. I cannot see the
> > success output in Florian's trace. Something like this:
> >
> > [    7.124879] hci_uart_bcm serial0-0: supply vbat not found, using
> > dummy regulator
> > [    7.131743] hci_uart_bcm serial0-0: supply vddio not found, using
> > dummy regulator
> > ...
> > [    7.517249] Bluetooth: hci0: BCM: chip id 107
> > [    7.517499] Bluetooth: hci0: BCM: features 0x2f
> > [    7.519757] Bluetooth: hci0: BCM4345C0
> > [    7.519768] Bluetooth: hci0: BCM4345C0 (003.001.025) build 0000
> > [    7.539495] Bluetooth: hci0: BCM4345C0 'brcm/BCM4345C0.hcd' Patch
> > ...
> > [    8.348831] Bluetooth: hci0: BCM43455 37.4MHz Raspberry Pi 3+
> > [    8.348845] Bluetooth: hci0: BCM4345C0 (003.001.025) build 0342
> >
> > I just want to make sure that 6.2 doesn't have a regression.
>
> My configuration uses hci_uart as a module, and it would always load
> fine, but I suppose I can make sure that even built-in this works
> properly. Give me a day or two to test that.

Thanks Stefan and Florian! I'll wait to see the results.

But based on my mental model of fw_devlink I don't expect BT to be
broken without this patch. If a device doesn't have fwnode set, it's
effectively invisible to fw_devlink. That could only affect consumers
of the device and not the device itself.

-Saravana

Re: [PATCH v1] serdev: Set fwnode for serdev devices

[Stefan Wahren]

Hi,

Am 03.03.23 um 18:22 schrieb Florian Fainelli:
> On 3/3/23 03:57, Stefan Wahren wrote:
>> Hi,
>>
>> Am 02.03.23 um 18:51 schrieb Florian Fainelli:
>>>
>>>
>>> On 3/2/2023 9:20 AM, Saravana Kannan wrote:
>>>> On Thu, Mar 2, 2023 at 9:01 AM Stefan Wahren 
>>>> <stefan.wahren@i2se.com> wrote:
>>>>>
>>>>> Hi Saravana,
>>>>>
>>>>> Am 02.03.23 um 03:35 schrieb Saravana Kannan:
>>>>>> This allow fw_devlink to do dependency tracking for serdev devices.
>>>>>>
>>>>>> Reported-by: Florian Fainelli <f.fainelli@gmail.com>
>>>>>> Link: 
>>>>>> https://lore.kernel.org/lkml/03b70a8a-0591-f28b-a567-9d2f736f17e5@gmail.com/
>>>>>> Cc: Stefan Wahren <stefan.wahren@i2se.com>
>>>>>> Signed-off-by: Saravana Kannan <saravanak@google.com>
>>>>>
>>>>> since this fixes an issue on Raspberry Pi 4, shouldn't this be 
>>>>> mentioned
>>>>> in the commit message and providing a Fixes tag?
>>>>
>>>> So RPi 4 was never creating a device links between serdev devices and
>>>> their consumers. The error message was just a new one I added and we
>>>> are noticing and catching the fact that serdev wasn't setting fwnode
>>>> for a device.
>>>>
>>>> I'm also not sure if I can say this commit "Fixes" an issue in serdev
>>>> core because when serdev core was written, fw_devlink wasn't a thing.
>>>> Once I add Fixes, people will start pulling this into stable
>>>> branches/other trees where I don't think this should be pulled into
>>>> older stable branches.
>>>
>>> That is kind of the point of Fixes: tag, is not it? It is 
>>> appropriate to list a commit that is not specific to serdev, but 
>>> maybe a particular point into the fw_devlink history. Given this did 
>>> not appear to have a functional impact, we could go without one.
>>
>> i was under the impression that this issue breaks at least Bluetooth 
>> on Raspberry Pi 4 because the driver is never probed. I cannot see 
>> the success output in Florian's trace. Something like this:
>>
>> [    7.124879] hci_uart_bcm serial0-0: supply vbat not found, using 
>> dummy regulator
>> [    7.131743] hci_uart_bcm serial0-0: supply vddio not found, using 
>> dummy regulator
>> ...
>> [    7.517249] Bluetooth: hci0: BCM: chip id 107
>> [    7.517499] Bluetooth: hci0: BCM: features 0x2f
>> [    7.519757] Bluetooth: hci0: BCM4345C0
>> [    7.519768] Bluetooth: hci0: BCM4345C0 (003.001.025) build 0000
>> [    7.539495] Bluetooth: hci0: BCM4345C0 'brcm/BCM4345C0.hcd' Patch
>> ...
>> [    8.348831] Bluetooth: hci0: BCM43455 37.4MHz Raspberry Pi 3+
>> [    8.348845] Bluetooth: hci0: BCM4345C0 (003.001.025) build 0342
>>
>> I just want to make sure that 6.2 doesn't have a regression.
>
> My configuration uses hci_uart as a module, and it would always load 
> fine, but I suppose I can make sure that even built-in this works 
> properly. Give me a day or two to test that.

okay, this is fine. From my point of view this is not necessary to test 
built-in.

I tested latest mainline with Raspberry Pi 4 (multi_v7_defconfig + 
ARM_LPAE) and there is no regression:

Tested-by: Stefan Wahren <stefan.wahren@i2se.com>

Thanks

Re: [PATCH v1] serdev: Set fwnode for serdev devices

[Florian Fainelli]

On 3/5/2023 7:00 AM, Stefan Wahren wrote:
> Hi,
> 
> Am 03.03.23 um 18:22 schrieb Florian Fainelli:
>> On 3/3/23 03:57, Stefan Wahren wrote:
>>> Hi,
>>>
>>> Am 02.03.23 um 18:51 schrieb Florian Fainelli:
>>>>
>>>>
>>>> On 3/2/2023 9:20 AM, Saravana Kannan wrote:
>>>>> On Thu, Mar 2, 2023 at 9:01 AM Stefan Wahren 
>>>>> <stefan.wahren@i2se.com> wrote:
>>>>>>
>>>>>> Hi Saravana,
>>>>>>
>>>>>> Am 02.03.23 um 03:35 schrieb Saravana Kannan:
>>>>>>> This allow fw_devlink to do dependency tracking for serdev devices.
>>>>>>>
>>>>>>> Reported-by: Florian Fainelli <f.fainelli@gmail.com>
>>>>>>> Link: 
>>>>>>> https://lore.kernel.org/lkml/03b70a8a-0591-f28b-a567-9d2f736f17e5@gmail.com/
>>>>>>> Cc: Stefan Wahren <stefan.wahren@i2se.com>
>>>>>>> Signed-off-by: Saravana Kannan <saravanak@google.com>
>>>>>>
>>>>>> since this fixes an issue on Raspberry Pi 4, shouldn't this be 
>>>>>> mentioned
>>>>>> in the commit message and providing a Fixes tag?
>>>>>
>>>>> So RPi 4 was never creating a device links between serdev devices and
>>>>> their consumers. The error message was just a new one I added and we
>>>>> are noticing and catching the fact that serdev wasn't setting fwnode
>>>>> for a device.
>>>>>
>>>>> I'm also not sure if I can say this commit "Fixes" an issue in serdev
>>>>> core because when serdev core was written, fw_devlink wasn't a thing.
>>>>> Once I add Fixes, people will start pulling this into stable
>>>>> branches/other trees where I don't think this should be pulled into
>>>>> older stable branches.
>>>>
>>>> That is kind of the point of Fixes: tag, is not it? It is 
>>>> appropriate to list a commit that is not specific to serdev, but 
>>>> maybe a particular point into the fw_devlink history. Given this did 
>>>> not appear to have a functional impact, we could go without one.
>>>
>>> i was under the impression that this issue breaks at least Bluetooth 
>>> on Raspberry Pi 4 because the driver is never probed. I cannot see 
>>> the success output in Florian's trace. Something like this:
>>>
>>> [    7.124879] hci_uart_bcm serial0-0: supply vbat not found, using 
>>> dummy regulator
>>> [    7.131743] hci_uart_bcm serial0-0: supply vddio not found, using 
>>> dummy regulator
>>> ...
>>> [    7.517249] Bluetooth: hci0: BCM: chip id 107
>>> [    7.517499] Bluetooth: hci0: BCM: features 0x2f
>>> [    7.519757] Bluetooth: hci0: BCM4345C0
>>> [    7.519768] Bluetooth: hci0: BCM4345C0 (003.001.025) build 0000
>>> [    7.539495] Bluetooth: hci0: BCM4345C0 'brcm/BCM4345C0.hcd' Patch
>>> ...
>>> [    8.348831] Bluetooth: hci0: BCM43455 37.4MHz Raspberry Pi 3+
>>> [    8.348845] Bluetooth: hci0: BCM4345C0 (003.001.025) build 0342
>>>
>>> I just want to make sure that 6.2 doesn't have a regression.
>>
>> My configuration uses hci_uart as a module, and it would always load 
>> fine, but I suppose I can make sure that even built-in this works 
>> properly. Give me a day or two to test that.
> 
> okay, this is fine. From my point of view this is not necessary to test 
> built-in.
> 
> I tested latest mainline with Raspberry Pi 4 (multi_v7_defconfig + 
> ARM_LPAE) and there is no regression:
> 
> Tested-by: Stefan Wahren <stefan.wahren@i2se.com>

Tested with making the BT drivers built-in with and without the patch 
and it still worked OK in both cases.
-- 
Florian