[U-Boot] [PATCHv4 3/3] ARM64: poplar: hi3798cv200: u-boot support for Poplar 96Boards

[Jorge Ramirez]

On 05/10/2017 11:31 PM, Simon Glass wrote:
>> There's 2 better options. You can define fixed clocks with
>> "fixed-clock" compatible and you already have infrastructure in u-boot
>> to use that. However, the upstream dts file already defines clocks, so
>> that doesn't really work here. The 2nd option is have a table of clock
>> ids and frequencies and register that list of clocks based on matching
>> the clock controller. You'd need a little bit of infrastructure to
>> support that, but otherwise a platform would just need to define a
>> table.
>>
>> It sounds like you are saying the first option isn't an option. The
>> second option adds another layer of pain - we are trying to avoid
>> having platform data.
>>
>> I'd prefer (in this order):
>>
>> 1. A clock driver
>>
>>
>> please could you explain the rationale for this request on this particular platform?
>>
>> And I mean for a platform where a clock driver will:
>>
>> 1. NOT access any hardware
>> 2.*only*  provide single hard-coded value (a compiled in frequency) for the pl01x driver.
>>
>> Consider also (another option) that the pl01x driver can be compiled without OF support and that said frequency can be provided by CONFIG.
>>
>> It is just that before adding layers of stub code I would like to understand the technical need when there is only one consumer (I don't want to pollute U-Boot's code base with code that is not providing value).
>>
>> What do we want to achieve by writing a SoC clock driver that hard-codes the frequency rate for the console?
>> what is the benefit of having such a driver and why is this not an overkill on this platform?
> For just one device I can accept that it is overkill. Once you have
> another device, or (e.g.) the ability to change clocks in U-Boot at
> run time, a clock driver makes sense.
>
>> 2. Use the existing clock-frequency property
>>
>>
>> yes, this I could understand.
>> And it doesn't even need to add a single line to the linux kernel dts files which would be imported from the linux kernel tree and keep unmodified.
> Then it sounds like this approach works for you?

ACK: this works for this platform (eMMC, USB host (net/storage) and UART).
if this were to change in the future extending with a clock driver as 
you said would make sense but I don't see this happening.

But to be clear, using "clock-frequency" would mean that I will have to 
modify serial_pl01x.c (replace "clock" with "clock-frequency") and also 
fix the device trees of all the current users of this driver - all of 
them modified their device trees to add u-boot's "clock" property.

do you concur with this?