[U-Boot] U-Boot of-platdata issue
Kever Yang
kever.yang at rock-chips.com
Tue Feb 14 01:09:01 UTC 2017
Hi Simon, Jaehoon,
On 02/13/2017 05:51 PM, Jaehoon Chung wrote:
> 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.
>
I don't think we will have a change in dts compatible for U-Boot dts,
because we will always using dts
file from kernel, so we will use it as is.
We can use "rockchip,rk3288-dw-mshc" for rk3288 and rk3399, here is the
document.
and for dw-mshc:
Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.txt
* compatible: should be
- "rockchip,rk2928-dw-mshc": for Rockchip RK2928 and following,
before RK3288
- "rockchip,rk3288-dw-mshc": for Rockchip RK3288
- "rockchip,rk3399-dw-mshc", "rockchip,rk3288-dw-mshc": for
Rockchip RK3399
For the compatible name, there had some discuss before, like this patch:
https://lists.gt.net/linux/kernel/2372182
So for the of-platdata, we can use "rockchip,rk3288-dw-mshc" to generate
the structure.
Thanks,
- Kever
>>> 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
>
>
>
More information about the U-Boot
mailing list