Re: [PATCH 5/7 v4] mmc: sh_mmcif: calculate best clock with parent clock

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Laurent

Thank you for your feedback

> > diff --git a/Documentation/devicetree/bindings/mmc/renesas,mmcif.txt
> > b/Documentation/devicetree/bindings/mmc/renesas,mmcif.txt index
> > 299081f..eb50f4e 100644
> > --- a/Documentation/devicetree/bindings/mmc/renesas,mmcif.txt
> > +++ b/Documentation/devicetree/bindings/mmc/renesas,mmcif.txt
> > @@ -14,6 +14,8 @@ Required properties:
> > 
> >  - clocks: reference to the functional clock
> > 
> > +- clk-range: parent clock range
> > +
> 
> I think the idea of a DT property to specify the admissible range for clock 
> frequencies is good. However, I have a couple of small comments regarding the 
> implementation.
> 
> First of all, this doesn't seem Renesas-specific to me (feel free to disagree, 
> but in that case the property should probably be called renesas,clk-range). 
> Wouldn't it make sense to specify the property in 
> Documentation/devicetree/bindings/clock/clock-bindings.txt instead ?
> 
> Then, I think the documentation should be clarified a bit, as "parent clock 
> range" is probably a bit vague for people who haven't followed the development 
> of this patch series. How about something like "range of admissible 
> frequencies for the input clock" ?
> 
> There's already a clock-ranges property with a totally different meaning. 
> That's quite confusing, it would thus be good to rename clk-range. How about 
> clock-freq-range or clock-frequency-range ?
> 
> Finally, if a DT node references several clocks in its clocks property, we 
> need a way to specify the range for each of them.

I didn't tell you about this, but actually I created v5 patch
(it is under test stage now, I will send it to ML soon)
which doesn't add new DT property.

This v4 added new property for clock frequencies, and it was Geert's idea.
This is just my opinion, and I don't know this is good match for DT,
but, this clock frequencies range depends on each SoC, and we are already
using SoC based "compatible" on DT. This means we can (should ?) get each SoC
specific feature (which is not related to each platform/board) from "compatible".
Thus, v5 patch gets clock frequency range from SoC based "compatible".

Thank you for your help, please check my next v5 patch.

Best regards
---
Kuninori Morimoto
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux