[U-Boot] [PATCH 04/10] arm: socfpga: arria10: Added clock manager and pin mux compat macro

Marek Vasut
2016-12-19T12:40:47Z

On 12/19/2016 11:31 AM, Chee, Tien Fong wrote:
> On Isn, 2016-12-19 at 11:04 +0100, Marek Vasut wrote:
>> On 12/19/2016 09:54 AM, Chee, Tien Fong wrote:
>>>
>>> On Isn, 2016-12-19 at 09:43 +0100, Marek Vasut wrote:
>>>>
>>>> On 12/19/2016 09:40 AM, Chee, Tien Fong wrote:
>>>>>
>>>>>
>>>>> On Isn, 2016-12-19 at 08:55 +0100, Marek Vasut wrote:
>>>>>>
>>>>>>
>>>>>> On 12/19/2016 05:10 AM, Chee, Tien Fong wrote:
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On Rab, 2016-12-07 at 14:54 +0100, Marek Vasut wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> On 12/07/2016 11:48 AM, Chee, Tien Fong wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On Sel, 2016-12-06 at 13:49 +0100, Marek Vasut wrote:
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> On 12/06/2016 08:52 AM, Chee Tien Fong wrote:
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> From: Tien Fong Chee <tien.fong.chee@intel.com>
>>>>>>>>>>>
>>>>>>>>>>> These compat macros would be used by clock manager
>>>>>>>>>>> and
>>>>>>>>>>> pin
>>>>>>>>>>> mux
>>>>>>>>>>> drivers
>>>>>>>>>>> to look the required HW info from DTS for hardware
>>>>>>>>>>> initialization.
>>>>>>>>>>>
>>>>>>>>>>> Signed-off-by: Tien Fong Chee <tien.fong.chee@intel
>>>>>>>>>>> .com
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>> Cc: Marek Vasut <marex@denx.de>
>>>>>>>>>>> Cc: Dinh Nguyen <dinguyen@kernel.org>
>>>>>>>>>>> Cc: Chin Liang See <chin.liang.see@intel.com>
>>>>>>>>>>> Cc: Tien Fong <skywindctf@gmail.com>
>>>>>>>>>>> ---
>>>>>>>>>>> include/fdtdec.h | 8 ++++++++
>>>>>>>>>>> lib/fdtdec.c | 2 ++
>>>>>>>>>>> 2 files changed, 10 insertions(+), 0 deletions(-)
>>>>>>>>>>>
>>>>>>>>>>> diff --git a/include/fdtdec.h b/include/fdtdec.h
>>>>>>>>>>> index 27887c8..68cb199 100644
>>>>>>>>>>> --- a/include/fdtdec.h
>>>>>>>>>>> +++ b/include/fdtdec.h
>>>>>>>>>>> @@ -155,6 +155,14 @@ enum fdt_compat_id {
>>>>>>>>>>> COMPAT_INTEL_BAYTRAIL_FSP_MDP, /*
>>>>>>>>>>> Intel
>>>>>>>>>>> FSP
>>>>>>>>>>> memory-
>>>>>>>>>>> down params */
>>>>>>>>>>> COMPAT_INTEL_IVYBRIDGE_FSP, /*
>>>>>>>>>>> Intel
>>>>>>>>>>> Ivy
>>>>>>>>>>> Bridge
>>>>>>>>>>> FSP
>>>>>>>>>>> */
>>>>>>>>>>> COMPAT_SUNXI_NAND, /* SUNXI
>>>>>>>>>>> NAND
>>>>>>>>>>> controller
>>>>>>>>>>> */
>>>>>>>>>>> + COMPAT_ALTERA_SOCFPGA_CLK, /*
>>>>>>>>>>> SoCFPGA
>>>>>>>>>>> Clock
>>>>>>>>>>> initialization */
>>>>>>>>>>> + COMPAT_ALTERA_SOCFPGA_PINCTRL_SINGLE,
>>>>>>>>>>> /
>>>>>>>>>>> *
>>>>>>>>>>> pinctrl-
>>>>>>>>>>> single */
>>>>>>>>>>> + COMPAT_ALTERA_SOCFPGA_H2F_BRG,
>>>>>>>>>>> /*
>>>>>>>>>>> Arria10
>>>>>>>>>>> hps2fpga bridge */
>>>>>>>>>>> + COMPAT_ALTERA_SOCFPGA_LWH2F_BRG, /*
>>>>>>>>>>> Arria10
>>>>>>>>>>> lwhps2fpga bridge */
>>>>>>>>>>> + COMPAT_ALTERA_SOCFPGA_F2H_BRG,
>>>>>>>>>>> /*
>>>>>>>>>>> Arria10
>>>>>>>>>>> fpga2hps bridge */
>>>>>>>>>>> + COMPAT_ALTERA_SOCFPGA_F2SDR0,
>>>>>>>>>>> /
>>>>>>>>>>> *
>>>>>>>>>>> Arria10
>>>>>>>>>>> fpga2SDRAM0 bridge */
>>>>>>>>>>> + COMPAT_ALTERA_SOCFPGA_F2SDR1,
>>>>>>>>>>> /
>>>>>>>>>>> *
>>>>>>>>>>> Arria10
>>>>>>>>>>> fpga2SDRAM1 bridge */
>>>>>>>>>>> + COMPAT_ALTERA_SOCFPGA_F2SDR2,
>>>>>>>>>>> /
>>>>>>>>>>> *
>>>>>>>>>>> Arria10
>>>>>>>>>>> fpga2SDRAM2 bridge */
>>>>>>>>>> Is all of this needed ? You're only adding two
>>>>>>>>>> entries in
>>>>>>>>>> the
>>>>>>>>>> FDTDEC
>>>>>>>>>> below.
>>>>>>>>>>
>>>>>>>>> This is to avoid compilation error, we have some
>>>>>>>>> functions
>>>>>>>>> ported
>>>>>>>>> from
>>>>>>>>> our internal branch, which using above COMPAT macro.
>>>>>>>>> Soon,
>>>>>>>>> in
>>>>>>>>> upcoming
>>>>>>>>> patches, we will need those functions and more entries
>>>>>>>>> will
>>>>>>>>> be
>>>>>>>>> added
>>>>>>>>> into FDTDEC below.
>>>>>>>> You can add the compat strings when you really need them.
>>>>>>>> Still,
>>>>>>>> with
>>>>>>>> DM, you shouldn't even need them AFAIK.
>>>>>>>>
>>>>>>> We have some drivers in these series of patches contain
>>>>>>> some
>>>>>>> COMPAT
>>>>>>> strings, without these compact strings, the compilation
>>>>>>> would
>>>>>>> fail
>>>>>>> due
>>>>>>> to error compact string is not defined.
>>>>>> All of them ? Mind you, with DM you should not need to add
>>>>>> those
>>>>>> at
>>>>>> all.
>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> I think having compact string
>>>>>>> would giving us flexbility to put our nodes where we want
>>>>>>> without
>>>>>>> worrying to break our existing codes?
>>>>>> Which existing codes ?
>>>>>>
>>>>> let say we have version A, we found the node based on COMPAT
>>>>> STRING.
>>>>> One day, we move the node to somewhere else in version B, we
>>>>> still
>>>>> can
>>>>> find them based on COMPAT STRING without checking the node
>>>>> path.
>>>>> What
>>>>> do you think?
>>>> I do not understand what you're trying to tell me here. The DM
>>>> core
>>>> will
>>>> walk the DT and bind drivers according to compat strings, so if
>>>> you ever move a node, it will still be bound.
>>>>
>>>> Also, you didn't answer my question -- which existing codes do
>>>> you
>>>> refer
>>>> to. Or is this some hypothetical concern ?
>>>>
>>> existing codes is hypothetical example, when someone move the node
>>> somewhere, the DM still can find the node based on COMPAT STRING.
>> Correct. If you only have hypothetical concerns, we'll deal with
>> those
>> when they become real.
>>
>>>
>>> Without COMPAT STRING, we need to change the driver code for the
>>> node
>>> path right?
>> Uh no, the driver must never encode a fixed DT path. It should match
>> on
>> the compat string and that's it. And the compat string is part of the
>> driver, not the FDT decoder. AFAIK, the strings in FDT decoder are
>> just
>> temporary helpers during the DT conversion of drivers.
>>
> Yeah, i have bridges reset driver in this series patches, and these
> drivers contains those COMPAT string. Without those COMPAT string,
> compilation would fail. It could be i missed to add those FDT entries,
> i can add those FDT entries in next version.

OK, then add those compat strings with the matching driver if and only
if it is needed.

--
Best regards,
Marek Vasut