[PATCH v4 14/21] arch: arm: socfpga: Add 'altr, sysmgr-syscon' for MMC node in device tree

[Marek Vasut]

On 3/11/20 8:06 AM, Ang, Chee Hong wrote:
> 
> 
>> -----Original Message-----
>> From: Simon Goldschmidt <simon.k.r.goldschmidt at gmail.com>
>> Sent: Wednesday, March 11, 2020 1:06 AM
>> To: Ang, Chee Hong <chee.hong.ang at intel.com>; u-boot at lists.denx.de
>> Cc: Marek Vasut <marex at denx.de>; See, Chin Liang <chin.liang.see at intel.com>;
>> Tan, Ley Foon <ley.foon.tan at intel.com>; Westergreen, Dalon
>> <dalon.westergreen at intel.com>; Gong, Richard <richard.gong at intel.com>
>> Subject: Re: [PATCH v4 14/21] arch: arm: socfpga: Add 'altr,sysmgr-syscon' for
>> MMC node in device tree

Can you please fix your mailer to avoid re-adding the entire header into
the message ?

>> Am 09.03.2020 um 10:07 schrieb chee.hong.ang at intel.com:
>>> From: Chee Hong Ang <chee.hong.ang at intel.com>
>>>
>>> In device tree for all socfpga platforms, a phandle to System Manager
>>> ('altr,sysmgr-syscon') is needed for MMC node to enable the MMC driver
>>> to configure the SDMMC's clock phase shift via System Manager driver
>>> (altera_sysmgr).
>>> This phandle specifies the offset of the SDMCC control register in
>>> System Manager, start of bit field for drvsel and start of bit field
>>> for smplsel.
>>>
>>> Signed-off-by: Chee Hong Ang <chee.hong.ang at intel.com>
>>> ---
>>>  arch/arm/dts/socfpga_agilex_socdk-u-boot.dtsi    | 1 +
>>>  arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts     | 1 +
>>>  arch/arm/dts/socfpga_arria5_socdk-u-boot.dtsi    | 1 +
>>>  arch/arm/dts/socfpga_cyclone5.dtsi               | 1 +
>>>  arch/arm/dts/socfpga_stratix10.dtsi              | 1 -
>>>  arch/arm/dts/socfpga_stratix10_socdk-u-boot.dtsi | 7 +++++++
>>>  arch/arm/dts/socfpga_stratix10_socdk.dts         | 2 --
>>
>> This looks strange. I would have expected you add the 'syscon' entry to the base
>> dtsi files (and to the ones in Linux, too, btw). But you're adding it to "-u-
>> boot.dtsi" files, too. Why?
> Where to add new device tree entry is rather confusing to me.
> Linux SDMMC driver doesn't set the SDMMC clock. So this only
> applicable to U-Boot only.

DT describes hardware, so U-Boot and Linux DTs should be ideally
identical. I would expect syscon, which is actual hardware, to be
applicable to both U-Boot and Linux (and other OSes too) ?

> I thought "-u-boot-dtsi" is the place where we should put those
> device tree entries that are only applicable to U-Boot only ?
That is more often used for things which are indeed U-Boot specific,
that is nodes which have u-boot, prefix and/or hardware bits which are
not yet part of Linux DT, but which _will_ be part of Linux DT once they
trickle through the upstream machinery.

[...]

-- 
Best regards,
Marek Vasut