[GIT PULL] clk: keystone: changes for 5.3 v2

[GIT PULL] clk: keystone: changes for 5.3 v2

[Tero Kristo]

Hi Stephen, Mike, Santosh,

Here's a 2nd take of the pull request for the clock changes for keystone 
SoC for 5.3. The only change compared to the v1 is to add the required 
drivers/firmware change in. This avoids the nasty dependency between the 
pull requests between the clock driver and firmware driver.

-Tero

---

The following changes since commit a188339ca5a396acc588e5851ed7e19f66b0ebd9:

   Linux 5.2-rc1 (2019-05-19 15:47:09 -0700)

are available in the git repository at:

   git://git.kernel.org/pub/scm/linux/kernel/git/kristo/linux 
tags/keystone-clk-for-5.3-v2

for you to fetch changes up to 81f4458c9c6998fcd37c427d16d76d4dba65d015:

   firmware: ti_sci: extend clock identifiers from u8 to u32 (2019-06-12 
14:45:00 +0300)

----------------------------------------------------------------
Keystone clk changes for 5.3 merge window.

- Add support for 32 bit clock IDs for sci-clks, this is needed
   for the new J721e SoC which has a few devices that have more than
   255 clocks associated to them.
- Clock probing done from DT by default instead of firmware side.
   Scanning clocks from DT is much faster than firmware, and also we
   can omit unnecessary clocks which saves even more time. This has been
   done in the interest of saving boot time.
- Remove the device tree node path from the registered sci-clk names.
   This mainly makes the debugfs interface more readable.
- Also contains a single drivers/firmware change which needs to go in
   via this pull-request; to support the 32bit clock IDs.

----------------------------------------------------------------
Tero Kristo (5):
       clk: keystone: sci-clk: cut down the clock name length
       clk: keystone: sci-clk: split out the fw clock parsing to own 
function
       clk: keystone: sci-clk: probe clocks from DT instead of firmware
       clk: keystone: sci-clk: extend clock IDs to 32 bits
       firmware: ti_sci: extend clock identifiers from u8 to u32

  drivers/clk/keystone/Kconfig           |  11 ++
  drivers/clk/keystone/sci-clk.c         | 239 
+++++++++++++++++++++++++++------
  drivers/firmware/ti_sci.c              | 115 +++++++++++-----
  drivers/firmware/ti_sci.h              |  63 +++++++--
  include/linux/soc/ti/ti_sci_protocol.h |  28 ++--
  5 files changed, 362 insertions(+), 94 deletions(-)
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [GIT PULL] clk: keystone: changes for 5.3 v2

[Stephen Boyd]

Quoting Tero Kristo (2019-06-12 04:56:15)
> Hi Stephen, Mike, Santosh,
> 
> Here's a 2nd take of the pull request for the clock changes for keystone 
> SoC for 5.3. The only change compared to the v1 is to add the required 
> drivers/firmware change in. This avoids the nasty dependency between the 
> pull requests between the clock driver and firmware driver.
> 
> -Tero
> 
> ---

Thanks. Pulled into clk-next. I guess we should increase the size of the
number of parents that can exist to be more than a u8? We're close to
getting there with the new way of specifying clk parents, so maybe we
should expand it to an unsigned int, but then we may need to optimize
finding parents when searching through all the parents of a clk.

Also, there isn't any quantification of how much better it is to scan DT
for all the clks that are used and only register those ones. It would be
nice to understand how much better it is to do that sort of scan vs.
just populating all clks at boot time. It may be useful to make the code
generic because NXP folks also want to populate clks from DT so maybe we
should provide this from the core framework somehow to ask providers to
register a particular clk or not. I haven't thought about it at all, but
it may come up that we totally rewrite this code in the future to be
shared outside of the TI clk driver.

FYI: I had to apply this patch on top to make sparse happy.

diff --git a/drivers/firmware/ti_sci.c b/drivers/firmware/ti_sci.c
index b417cef35769..92b77d38dd1f 100644
--- a/drivers/firmware/ti_sci.c
+++ b/drivers/firmware/ti_sci.c
@@ -2402,12 +2402,13 @@ devm_ti_sci_get_of_resource(const struct ti_sci_handle *handle,
 	if (!res)
 		return ERR_PTR(-ENOMEM);
 
-	res->sets = of_property_count_elems_of_size(dev_of_node(dev), of_prop,
+	ret = of_property_count_elems_of_size(dev_of_node(dev), of_prop,
 						    sizeof(u32));
-	if (res->sets < 0) {
+	if (ret < 0) {
 		dev_err(dev, "%s resource type ids not available\n", of_prop);
-		return ERR_PTR(res->sets);
+		return ERR_PTR(ret);
 	}
+	res->sets = ret;
 
 	res->desc = devm_kcalloc(dev, res->sets, sizeof(*res->desc),
 				 GFP_KERNEL);


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [GIT PULL] clk: keystone: changes for 5.3 v2

[Tero Kristo]

On 25/06/2019 04:17, Stephen Boyd wrote:
> Quoting Tero Kristo (2019-06-12 04:56:15)
>> Hi Stephen, Mike, Santosh,
>>
>> Here's a 2nd take of the pull request for the clock changes for keystone
>> SoC for 5.3. The only change compared to the v1 is to add the required
>> drivers/firmware change in. This avoids the nasty dependency between the
>> pull requests between the clock driver and firmware driver.
>>
>> -Tero
>>
>> ---
> 
> Thanks. Pulled into clk-next. I guess we should increase the size of the
> number of parents that can exist to be more than a u8? We're close to
> getting there with the new way of specifying clk parents, so maybe we
> should expand it to an unsigned int, but then we may need to optimize
> finding parents when searching through all the parents of a clk.

For now, this is not an issue with TI SoC:s at least, I think we only 
have like 64 parents at max for muxes.

> Also, there isn't any quantification of how much better it is to scan DT
> for all the clks that are used and only register those ones. It would be
> nice to understand how much better it is to do that sort of scan vs.
> just populating all clks at boot time.

I haven't done measurements lately, but it provides pretty drastic 
improvement. On am65x for example, it cuts the scan time from bit more 
than 1 second to couple of hundred milliseconds. I don't have 
measurements for the new j721e SoC, but I would believe the improvement 
is even more with that one.

> It may be useful to make the code
> generic because NXP folks also want to populate clks from DT so maybe we
> should provide this from the core framework somehow to ask providers to
> register a particular clk or not. I haven't thought about it at all, but
> it may come up that we totally rewrite this code in the future to be
> shared outside of the TI clk driver.

It might also be worth thinking whether some sort of lazy clock probe 
would be possible... now we register everything at one go, but would it 
be possible to only register / reparent clocks once they are actually 
requested by some driver?

> 
> FYI: I had to apply this patch on top to make sparse happy.

Ok looks good to me, thanks.

-Tero

> 
> diff --git a/drivers/firmware/ti_sci.c b/drivers/firmware/ti_sci.c
> index b417cef35769..92b77d38dd1f 100644
> --- a/drivers/firmware/ti_sci.c
> +++ b/drivers/firmware/ti_sci.c
> @@ -2402,12 +2402,13 @@ devm_ti_sci_get_of_resource(const struct ti_sci_handle *handle,
>   	if (!res)
>   		return ERR_PTR(-ENOMEM);
>   
> -	res->sets = of_property_count_elems_of_size(dev_of_node(dev), of_prop,
> +	ret = of_property_count_elems_of_size(dev_of_node(dev), of_prop,
>   						    sizeof(u32));
> -	if (res->sets < 0) {
> +	if (ret < 0) {
>   		dev_err(dev, "%s resource type ids not available\n", of_prop);
> -		return ERR_PTR(res->sets);
> +		return ERR_PTR(ret);
>   	}
> +	res->sets = ret;
>   
>   	res->desc = devm_kcalloc(dev, res->sets, sizeof(*res->desc),
>   				 GFP_KERNEL);
> 

--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [GIT PULL] clk: keystone: changes for 5.3 v2

[Stephen Boyd]

Quoting Tero Kristo (2019-06-25 03:33:50)
> On 25/06/2019 04:17, Stephen Boyd wrote:
> > Quoting Tero Kristo (2019-06-12 04:56:15)
> >> Hi Stephen, Mike, Santosh,
> >>
> >> Here's a 2nd take of the pull request for the clock changes for keystone
> >> SoC for 5.3. The only change compared to the v1 is to add the required
> >> drivers/firmware change in. This avoids the nasty dependency between the
> >> pull requests between the clock driver and firmware driver.
> >>
> >> -Tero
> > 
> > Thanks. Pulled into clk-next. I guess we should increase the size of the
> > number of parents that can exist to be more than a u8? We're close to
> > getting there with the new way of specifying clk parents, so maybe we
> > should expand it to an unsigned int, but then we may need to optimize
> > finding parents when searching through all the parents of a clk.
> 
> For now, this is not an issue with TI SoC:s at least, I think we only 
> have like 64 parents at max for muxes.

Ok. It doesn't sound like a priority then.

> 
> > Also, there isn't any quantification of how much better it is to scan DT
> > for all the clks that are used and only register those ones. It would be
> > nice to understand how much better it is to do that sort of scan vs.
> > just populating all clks at boot time.
> 
> I haven't done measurements lately, but it provides pretty drastic 
> improvement. On am65x for example, it cuts the scan time from bit more 
> than 1 second to couple of hundred milliseconds. I don't have 
> measurements for the new j721e SoC, but I would believe the improvement 
> is even more with that one.

Cool. Thanks for the numbers.

> 
> > It may be useful to make the code
> > generic because NXP folks also want to populate clks from DT so maybe we
> > should provide this from the core framework somehow to ask providers to
> > register a particular clk or not. I haven't thought about it at all, but
> > it may come up that we totally rewrite this code in the future to be
> > shared outside of the TI clk driver.
> 
> It might also be worth thinking whether some sort of lazy clock probe 
> would be possible... now we register everything at one go, but would it 
> be possible to only register / reparent clocks once they are actually 
> requested by some driver?

Sure. Ideally the optimization isn't vendor driver specific.


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel