2014-02-28 17:06 GMT+08:00 Romain Izard <[email protected]>: > Hello Barry, > > 2014-02-28 4:01 GMT+01:00 Barry Song <[email protected]>: >> >> Hi Romain, >> >> do you have a real user scenarios for this 32KHz? as the 1st step, i >> want a clean and general enough pwm driver, if there is any special >> requirement, i want to execute a "demanding page" when the "page >> fault" happened as this will make the whole flow more smooth. that >> means we make it lazy by incremental patches. but if you do want to >> use it for the moment, it is a "page fault" now. so we can have it >> immediately and it is better you can help to double-test :-) > > My use case is to connect a Bluetooth module, in fact with a CSR chip. > For testing, I will try to see what I can do, as for now my boards are not > running with the mainline. But I'm very interested in transitioning on the > mainline in due time, so that's the reason I keep an eye on the mailing > lists. Conversely, it's not critical at all to get the feature in right now, > but > it should not be impossible to do so in the future, especially when taking > the 'stable interface' aspect of device tree bindings into account. > >> 2014-02-27 18:51 GMT+08:00 Romain Izard <[email protected]>: >>> Perhaps this can be linked with the missing "clock-names" property, as this >>> will make it possible to add multiple functional clocks, with "iclk" >>> matching >>> the interface clock (i.e. pwmc), and "fclk0", "fclk1", etc. matching >>> all supported >>> functional clocks. >>> >>> With only the 26 MHz clock, the node would look like: >>> >>> pwm: pwm@b0130000 { >>> compatible = "sirf,prima2-pwm"; >>> #pwm-cells = <2>; >>> reg = <0xb0130000 0x10000>; >>> clocks = <&clks 21>, <&clks 1>, <&clks 0>; >>> clock-names = "iclk", "fclk0", "fclk3"; >>> }; >>> > > Aargh. I meant this: > > pwm: pwm@b0130000 { > compatible = "sirf,prima2-pwm"; > #pwm-cells = <2>; > reg = <0xb0130000 0x10000>; > clocks = <&clks 21>, <&clks 1>; > clock-names = "iclk", "fclk0"; > }; > >>> The result would look like this with both the 26 MHz and the 32 kHz clocks: >>> >>> pwm: pwm@b0130000 { >>> compatible = "sirf,prima2-pwm"; >>> #pwm-cells = <2>; >>> reg = <0xb0130000 0x10000>; >>> clocks = <&clks 21>, <&clks 1>, <&clks 0>; >>> clock-names = "iclk", "fclk0", "fclk3"; >>> }; >>> >>> And with all possible clocks: >>> >>> pwm: pwm@b0130000 { >>> compatible = "sirf,prima2-pwm"; >>> #pwm-cells = <2>; >>> reg = <0xb0130000 0x10000>; >>> clocks = <&clks 21>, <&clks 1>, <&clks 2>, <&clks 3>, <&clks >>> 0>, <&clks 4>; >>> clock-names = "iclk", "fclk0", "fclk1", "fclk2", "fclk3", "fclk4"; >>> }; >>> >>> The code in the probe function would interpret the clock-name to build >>> the clock/index mapping table. This would not change a lot the binding you >>> proposed, as you still can describe only one functional clock, but it >>> describes >>> which index in the configuration register is linked to the proposed clock. >>> >> >> yes. very clear. the only two things left >> >> 1. fclk should be named for pwm not for rtc, osc and pll, so the >> names might be ugly by fclk0~N. >> > For me, it's important to keep the number in the name of the clock as it is > the source of the information for binding the device tree clock on one side > and the clock source index in the selection register on the other side. If > we do not do it this way, we cannot easily handle new clocks in the future > in the binding. > > On the other hand, I strongly dislike the current clock specification > as <&clks #x>. > It would be probably a good thing to add a header linking the numbers > to symbols, > and we would then get: > > clocks = <&clks SIRFCLK_A6_PWM>, > <&clks SIRFCLK_A6_OSC>, > <&clks SIRFCLK_A6_PLL1>, > <&clks SIRFCLK_A6_PLL2>, > <&clks SIRFCLK_A6_RTC>, > <&clks SIRFCLK_16_PLL3>; > clock-names = "iclk", "fclk0", "fclk1", "fclk2", "fclk3", "fclk4"; > > Note that as the binary output of the device tree would not change with this, > we > do not need to worry about backwards compatibility in this precise case.
this pwm controller is prima2-compatible, so it is actually a pwm controller highly bound with the special SoC. really strange things are we actually care about the clock types. we actually can't think fclk0~fclk4 as same things. for pll, i think it is buggy to use. for rtc, it only service special target for providing bypass 32KHz clock. if we look this controller a separate IP, we need to look 5 clock source to be coequal. but it is not the real case. i think people will hate the things that we have a separate bypass mode for fclk3, why it is 3 but not 2, 1, 4 and 5? that is why i move to a MACRO 26MHz in the original codes as i think it is a prima2 controller but not a controller used by prima2 even though we always want a IP module to be a IP module suitable for all SoCs. > >> 2. the driver need to manage all of the clocks. yes, it can. but it >> might be ugly again. we can either request the clk at the time of pwm >> configuration, or get all directy in probe and maintain an array. > > For me, what is important right now is to get the binding right. If the driver > cannot do everything the binding allows, this is not a problem. But we define > a binding that cannot easily be expanded, we will have problems when we > need the new features. > >> i am thinking what is the best way for it. if we do that by an >> incremental patch, it might be more concentrated and understandable. i think bypass mode for rtc will be included immediately since there is a real user. > > I'm all for this as well, that why for me the first syntax (corrected > up there) should > be valid. This way, you do not need to integrate the bypass support in > this step, > but you can add it later. > > Best regards, > -- > Romain Izard -barry -- To unsubscribe from this list: send the line "unsubscribe linux-pwm" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
