[U-Boot] [PATCH 5/9] clock: add Tegra186 clock driver

[Simon Glass]

Hi Stephen,

On 1 August 2016 at 09:46, Stephen Warren <swarren at wwwdotorg.org> wrote:
> On 07/31/2016 07:02 PM, Simon Glass wrote:
>>
>> Hi Stephen,
>>
>> On 27 July 2016 at 15:24, Stephen Warren <swarren at wwwdotorg.org> wrote:
>>>
>>> From: Stephen Warren <swarren at nvidia.com>
>>>
>>> In Tegra186, on-SoC clocks are manipulated using IPC requests to the BPMP
>>> (Boot and Power Management Processor). This change implements a driver
>>> that does that. A tegra/ sub-directory is created to follow the existing
>>> pattern. It is unconditionally selected by CONFIG_TEGRA186 since
>>> virtually
>>> any Tegra186 build of U-Boot will need the feature.
>
>
>>> diff --git a/drivers/clk/tegra/tegra186-clk.c
>>> b/drivers/clk/tegra/tegra186-clk.c
>
>
>>> +static ulong tegra186_clk_get_rate(struct clk *clk)
>>> +{
>>> +       struct mrq_clk_request req;
>>> +       struct mrq_clk_response resp;
>>> +       int ret;
>>> +
>>> +       debug("%s(clk=%p) (dev=%p, id=%lu)\n", __func__, clk, clk->dev,
>>> +             clk->id);
>>> +
>>> +       req.cmd_and_id = (CMD_CLK_GET_RATE << 24) | clk->id;
>>> +
>>> +       ret = tegra186_bpmp_call(clk->dev->parent, MRQ_CLK,
>>> +                                &req, sizeof(req), &resp, sizeof(resp));
>>
>>
>> Isn't his a MISC driver? Perhaps you can add a new method to
>> UCLASS_MISC matching your requirements here?
>
>
> The core BPMP driver is a MISC driver, yes.
>
> I don't see any advantage of making this call something that the MISC uclass
> supports directly. This function is an internal implementation detail of the
> BPMP, and certainly not something that every single MISC driver (for any SoC
> for any HW module) would implement. If we did add direct support to the MISC
> uclass, then the MISC "ops" structure would basically grow forever since
> every single SoC's/HW's internal function calls would be added to it. This
> would just bloat it up unnecessarily, and I don't see any advantage to
> offset the disadvantage of that bloat.

I'm really just suggesting that you could add one more method to the
misc uclass, which does a simultaneous write and read.

>
> FWIW, the Linux MFD (Multi-Function Devices) stack typically has "child"
> (sub-)devices call custom APIs like this on the "parent"/container.

Yes, I'm just trying to encourage use of the driver-model features -
one of the design goals is to expose otherwise private driver
interfaces.

Regards,
Simon