Re: [PATCH 0/4] net: ti: am65-cpsw-nuss: Fix DT binding handling of pinctrl

[Roger Quadros <rogerq@kernel.org>]

On 20/07/2023 17:12, Maxime Ripard wrote:
> Hi
> 
> Sorry, I didn't receive Roger mail so I'll reply here
> 
> On Thu, Jul 20, 2023 at 09:00:19AM -0500, Nishanth Menon wrote:
>> On 16:56-20230720, Roger Quadros wrote:
>>> Hi,
>>>
>>> On 20/07/2023 16:33, Ravi Gunasekaran wrote:
>>>>
>>>>
>>>> On 7/20/2023 3:25 PM, Maxime Ripard wrote:
>>>>> Hi,
>>>>>
>>>>> This series is based on:
>>>>> https://lore.kernel.org/all/20230713072019.3153871-1-nm@ti.com/
>>>>>
>>>>> It fixes the issue of Linux booting from the DT embedded by U-boot. The
>>>>> main issue there is that U-Boot doesn't handle the MDIO child node that
>>>>> might have resources attached to it.
>>>>>
>>>>> Thus, any pinctrl configuration that could be attached to the MDIO
>>>>> child node is effectively ignored. Unfortunately, starting with 6.5-rc1,
>>>>> Linux does just that.
>>>
>>> I didn't get this part. Linux does not ignore pinctrl configuration attached
>>> to MDIO child node. What changed in 6.5-rc1?
> 
> Linux doesn't ignore it, but Linux started putting pinctrl configuration
> on the MDIO node, which is ignored by U-Boot.

Linux always had MDIO pinctrl configuration in the MDIO node, way before 6.5-rc1.
Let's not mention 6.5-rc1 here as it gives an indication that something in 6.5-rc1
caused this issue. ;)

Earlier, u-boot didn't enable the MDIO node. With Nishanth's sync it does which
is fine, but duplicating the MDIO pinctrl node in the CPSW node causes problems
on Linux.

I see you reverted that change in patch 3, but probably Nishanth can avoid it if we
got this route.

> 
>>>>> This was solved by duplicating the pinctrl configuration onto the MAC
>>>
>>> If I remember right, there is no driver model driver for MDIO in u-boot and
>>> adding the mdio pinctrl into CPSW DT node was a hack in u-boot.
> 
> Yeah, but we now have in the U-Boot DT two nodes referencing the same
> pinctrl configuration: the CPSW and the MDIO node. Linux then tries to
> assign that configuration to both devices and it fails.

Understood.
> 
>>>>> device node. Unfortunately, this doesn't work for Linux since now it has
>>>>> two devices competing for the same pins.
>>>
>>> What has really changed here is that you are passing u-boot's device
>>> tree to Linux.
> 
> I didn't change anything. This is the default boot process if you don't
> provide a DT in the ESP partition.
> 

OK.
>>> This has nothing to do with 6.5-rc1 right?
> 
> The issue started to occur with Nishanth patch to sync with Linux
> 6.5-rc1 device trees, so there's definitely a connection.
> 
>>> I suppose your solution is still a hack but of lesser evil than
>>> duplicating MDIO pinctrl in CPSW node.
>>>
>>> The proper solution would be to implement driver model for the
>>> davinci MDIO driver. Siddharth has been working on this. If that is
>>> close to ready we should just use that instead.
>>
>> But this allows for a cleaner device tree while the driver can be fixed
>> up independently, correct? Unfortunately, Siddharth's driver model work,

Yes. MDIO pinctrl no longer needs to be duplicated in CPSW node at u-boot.

>> from what I understand, is around 2024 timeframe.. which is probably not
>> something that helps us in the community at this point.

OK, then we need to resort to some temporary solution like this then.
> 
> Yeah, at the moment I have to choose between getting the MMC to work in
> Linux or the Ethernet ports. The former is working thanks to your patch,
> the latter is broken because of it. Ideally I'd like both :)
> 

-- 
cheers,
-roger