Am Dienstag, 4. Juni 2013, 21:22:43 schrieb Mike Turquette: > Quoting Matt Sealey (2013-06-04 10:39:53) > > > On Tue, Jun 4, 2013 at 12:11 PM, Stephen Boyd <sb...@codeaurora.org> wrote: > > > On 06/03/13 10:53, Mike Turquette wrote: > > >> +Required properties: > > >> +- compatible : shall be "divider-clock". > > >> +- #clock-cells : from common clock binding; shall be set to 0. > > >> +- clocks : link to phandle of parent clock > > >> +- reg : base address for register controlling adjustable divider > > >> +- mask : arbitrary bitmask for programming the adjustable divider > > >> + > > >> +Optional properties: > > >> +- clock-output-names : From common clock binding. > > >> +- table : array of integer pairs defining divisors & bitfield values > > >> +- shift : number of bits to shift the mask, defaults to 0 if not > > >> present +- index_one : valid divisor programming starts at 1, not > > >> zero +- index_power_of_two : valid divisor programming must be a > > >> power of two +- allow_zero : implies index_one, and programming zero > > >> results in + divide-by-one > > > > > > It's preferred that property names have dashes instead of underscores. > > > > I think I have a suggestion or two here.. > > > > I mailed one to Mike earlier, I have had some mailing list problems > > which I think are solved now.. > > > > If you provide a mask for programming the divider, surely the "shift" > > property is therefore redundant. Unless some device has a completely > > strange way of doing things and uses two seperated bitfields for > > programming in the same register, the shift value is simply a function > > of ffs() on the mask, isn't it? It is nice to put the information > > specifically in the device tree, but where a shift is not specified > > it'd probably be a good idea to go about deriving that value that way > > anyway, right, and if we're doing that.. why specify it? > > Shift is optional. Also I think the integer values in a DTS are > 32-bits, so if some day someone had a clock driver that needed to write > to a 64-bit register then shift might be useful there, combined with a > 32-bit mask. Or perhaps dtc will have a 64-bit value for that case? I > don't know. > > I certainly do see your point about only using the mask, and the > original clock basic types did this a while back before the shift+width > style came into vogue. > > If other reviewers also want to use a pure 32-bit mask and no shift then > I'll drop it in the next round.
I would object to dropping the shift :-) If you look at the two clk extensions in my rockchip patches you can see that they use a different regiment to set values. Using the lower 16 bits to set the value and the upper 16 bit to mark which values are changed, i.e. (mask << shift + 16) | (val << shift). (I'm not sure, if the naming or solution could be improved though) Heiko -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
