Re: [PATCH v4 4/5] clk: qcom: Add RPM clock controller driver

2015-11-27 Thread Bjorn Andersson
On Mon, Nov 23, 2015 at 4:59 PM, Stephen Boyd  wrote:
> On 11/21, Bjorn Andersson wrote:
>> On Fri 20 Nov 16:39 PST 2015, Stephen Boyd wrote:
[..]
>> Do you foresee that there will be an implementation of the generic rpmcc
>> or is it just a way to "standardize" the dt binding?
>>
>
> I don't see any problem with implementing the RPM clock
> controller as one file or two files (one for platform bus based
> RPM modules and one for SMD bus RPM modules). The compatible can
> be the same for both struct driver instances, while the bus will
> pick the right driver. I suspect we'll need SoC specific
> compatibles though to export the right set of clocks, so having
> the generic compatible is mostly to find these rpm clock
> controllers so that we know to skip registering the XO clock from
> the GCC driver and not some generic implementation of the driver.
>

Ahh, now I get it. So the generic rpmcc to be able to check if we have
a rpmcc instance and then the specific for the actual implementation.

I'm +1 on that.

> There's probably a subset of the clocks that's always the same
> between devices, so if we had to we could match the generic
> compatible and provide limited functionality.
>

After looking at a couple of platforms I don't think it's worth the
effort of having a common list of rpm clocks. My suggestion is that we
just continue with the suggested approach (having platform specific
rpmcc defines and tables)

Regards,
Bjorn
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v8 11/13] clk: qcom: gdsc: Use PM clocks to control gdsc clocks

2015-11-27 Thread Rajendra Nayak
Hi Stephen,

>> +static int gdsc_attach(struct generic_pm_domain *domain, struct device
>> *dev)
>> +{
>> +int ret, i = 0, j = 0;
>> +struct gdsc *sc = domain_to_gdsc(domain);
>> +struct of_phandle_args clkspec;
>> +struct device_node *np = dev->of_node;
>> +
>> +if (!sc->clock_count)
>> +return 0;
>> +
>> +ret = pm_clk_create(dev);
>> +if (ret) {
>> +dev_dbg(dev, "pm_clk_create failed %d\n", ret);
>> +return ret;
>> +}
>> +
>> +sc->clks = devm_kcalloc(dev, sc->clock_count, sizeof(sc->clks),
>> +   GFP_KERNEL);
>> +if (!sc->clks)
>> +return -ENOMEM;
>> +
>> +while (!of_parse_phandle_with_args(np, "clocks", "#clock-cells", i,
>> +   &clkspec)) {
>> +if (match(clkspec.args[0], sc->clocks, sc->clock_count)) {
>
> I'm lost. I was hoping we could just make up a clkspec on the
> stack and pass it over to of_clk_get_from_provider() without
> having to go through the np of the client device. The point being
> to avoid forcing this code from knowing about the consumer
> binding or connection name choice for each device. Instead, it
> assumes that it's a #clock-cells=<1> binding and gets the clocks
> by passing the 1 cell data without calling
> of_parse_phandle_with_args().
>
> Now, one downside of that approach is that it's DT centric (also
> of_clk_get_from_provider() is not an exported symbol yet). So I'm
> really starting to lean towards exposing __clk_create_clk() (or
> some better named "provider" function) that will allow clk
> providers to turn their clk_hw structure into a struct clk
> pointer. That avoids the DT centric design, and avoids binding
> the provider to the connection ids too.

Stephen, I started to relook at these patches, avoiding the DT centric
design and implementing a clk helper API as you suggested above.

While this would work for GDSCs with just one device, its hard to scale
if we ever run into GDSCs with multiple devices (In which case you
need to know which device within the GDSC needs which clocks)

Do you think its a fair limitation (one device per gdsc) to live with?
because I can't seem to figure how a non DT centric design would otherwise
work.

regards,
Rajendra

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html