[U-Boot] U-Boot of-platdata issue

[Jaehoon Chung]

On 02/13/2017 06:23 PM, Kever Yang wrote:
> Hi Simon,
> 
> On 01/16/2017 12:15 PM, Simon Glass wrote:
>> Hi Kever,
>>
>> On 15 January 2017 at 18:28, Kever Yang <kever.yang at rock-chips.com> wrote:
>>> Hi Simon,
>>>
>>>      I met two issue when using of-platdata
>>>
>>> 1. compitable name with '.'
>>> I get compile error as below:
>>> In file included from include/dt-structs.h:16:0,
>>>                   from spl/dts/dt-platdata.c:3:
>>> include/generated/dt-structs.h:26:35: error: expected identifier or ‘(’
>>> before numeric constant
>>>   struct dtd_rockchip_rk3399_sdhci_5.1 {
>>>                                     ^
>>> spl/dts/dt-platdata.c:41:42: error: expected identifier or ‘(’ before
>>> numeric constant
>>>   static struct dtd_rockchip_rk3399_sdhci_5.1 dtv_sdhci_at_fe330000 = {
>>>                                            ^
>>> spl/dts/dt-platdata.c:55:15: error: ‘dtv_sdhci_at_fe330000’ undeclared here
>>> (not in a function)
>>>    .platdata = &dtv_sdhci_at_fe330000,
>>>                 ^
>>> make[2]: *** [spl/dts/dt-platdata.o] Error 1
>>> make[1]: *** [spl/u-boot-spl] Error 2
>>> make: *** [__build_one_by_one] Error 2
>>>
>>> The dts node starts like this:
>>>          sdhci: sdhci at fe330000 {
>>>                  u-boot,dm-pre-reloc;
>>>                  compatible = "rockchip,rk3399-sdhci-5.1",
>>> "arasan,sdhci-5.1";
>>> ...
>> That just involves replacing '.' with '_'. I sent a patch.
>>
>>> 2. multi compatible name
>>> When a dts node have more than one compatible name, which is prefer to use?
>>> for example, we have two dwmmc compatible name in rk3399, the tool is using
>>> the first one,
>>> while the source code using the last one.
>>>
>>> "drivers/mmc/rockchip_dw_mmc.c"
>>>   23 struct rockchip_mmc_plat {
>>>   24 #if CONFIG_IS_ENABLED(OF_PLATDATA)
>>>   25         struct dtd_rockchip_rk3288_dw_mshc dtplat;
>>>   26 #endif
>>>   27         struct mmc_config cfg;
>>>   28         struct mmc mmc;
>>>   29 };
>>> ...
>>> dts node
>>>          sdmmc: dwmmc at fe320000 {
>>>                 compatible = "rockchip,rk3399-dw-mshc",
>>>                               "rockchip,rk3288-dw-mshc";
>> I'm not sure of the best solution here (other than putting more
>> on-chip SRAM in your devices hint hint :-)
>>
>> One option is something like:
>>
>> struct rockchip_mmc_plat {
>> #if CONFIG_IS_ENABLED(OF_PLATDATA)
>> #ifdef CONFIG_ROCKCHIP_RK3288
>>          struct dtd_rockchip_rk3288_dw_mshc dtplat;
>> #elif defined(CONFIG_ROCKCHIP_RK399)
>>          struct dtd_rockchip_rk399_dw_mshc dtplat;
>> #endif
>> #endif
>>
>> Obviously we don't want that as it is putting SoC-specific stuff in the driver.
>>
>> IMO the compatible strings are being misused a bit. Can there not be a
>> compatible string which is common to all rockchip devices which use
>> this IP? Something like "rockchip,dw-mshc-v1"? Then you can avoid
>> adding a new compatible string every time you use the same IP in a
>> device.
> 
> Agree, but... this is from kernel, we can't control it unless all kernel maintainers
> have the same idea.

does it use just "rockchip,dw-mshc"? Maybe this can be common compatible for rockchip.
If it needs add the other compatible in future, it should be added the specific compatible at that time.

>>
>> Another option would be for dtoc to #define each compatible string to
>> the first one. If you think that would work, I could do a patch.
> 
>     I think we can try with the first one in the driver for of-platdata,
> this would have problem for the first compatible name is different but
> they share the same driver, like syscon. For syscon, you have resolved with
> a special dirver, not sure if other driver has the same problem.
> 
>     Could you help to make a patch for this solution?
> 
> Thanks,
> - Kever
>>
>> Regards,
>> Simon
>>
>>
>>
> 
> 
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot