Re: [PATCH v5 3/3] ARM: dts: igep00x0: add wl18xx bindings
* Eliad Peller el...@wizery.com [150310 10:01]: On Tue, Mar 10, 2015 at 6:18 PM, Tony Lindgren t...@atomide.com wrote: * Eliad Peller el...@wizery.com [150310 09:11]: On Tue, Mar 10, 2015 at 5:52 PM, Arnd Bergmann a...@arndb.de wrote: On Tuesday 10 March 2015 16:31:33 Eliad Peller wrote: On Tue, Mar 10, 2015 at 4:11 PM, Arnd Bergmann a...@arndb.de wrote: On Tuesday 10 March 2015 13:00:19 Eliad Peller wrote: On Tue, Mar 10, 2015 at 12:49 AM, Tony Lindgren t...@atomide.com wrote: I was expecting you to remove all calls to legacy_init_wl12xx from this file, including the ones for wl12xx aside from the wl18xx ones you removed, but if that's enough to clean out the platform_data handling from the wlcore driver, it's good enough as a start. not sure i'm following - can you elaborate? i'll summarize the way i see it. please correct me if i'm wrong. both wl18xx and wl12xx use the platform data to get the irq number. wl12xx (only) also needs some additional clock definitions to be passed. there's currently some issue with specifying some the of clock sources, so i preferred starting only with (the simpler) wl18xx bindings. for platforms with wl18xx, we can remove the pdata-quirk, as all the data (i.e. irq) can be passed by the new DT bindings. however, for platforms with wl12xx, we still need to pass the clock definitions (along with the irq), so we have to keep legacy_init_wl12xx for the time being (and that's also why we have to currently keep the platform_data handling in the wlcore driver) do you have something else in mind? I think what Arnd is saying is we've now removed all the wl12xx using legacy platforms, so all of them can boot with just data from dts. Right, that was my idea. I don't think that's the case (unless i'm missing something). e.g. there's still pdata-quirk for compulab,omap3-sbc-t3730 which initializes wl12xx device. This one is just like the igep0030, as Tony was saying: the board boots from device tree already, so now that we have a binding for it, we can remove the wl12xx_set_platform_data() for it. i think the wl12xx_set_platform_data() name created some confusion - it is used to pass platform data for both wl12xx and wl18xx devices. (this confusion is all around the wlcore driver as well, due to the code evolution) the binding i added is for wl18xx only (there is no wl12xx binding yet). the remaining boards, AFAICT, have wl12xx (rather than wl18xx) cards. so i don't see how we can remove these wl12xx_set_platform_data() calls before we have wl12xx bindings in-place as well. What is missing for that binding then? I keep getting confused here, but I thought that they share the implementation that looks at the platform data. they both get the same wl12xx_platform_data struct, but only wl12xx cares about the clocks-related fields. the bindings i added parses only the irq. (Luca tried previously to upstream wl12xx DT support along with the required clock DT changes, but got some rejections, mainly wrt. clock stuff. e.g. http://thread.gmane.org/gmane.linux.kernel/1520752 that's why i preferred starting with easier wl18xx bindings only) I believe we did not have clock bindings back then, now it's simple to get the clock. If it's some internal clock to the wl12xx, then that's a different story, it should be just hidden behind a compatible flag then. i'm really not familiar with the common clock framework, but there still doesn't seem to be a way to determine whether a clock is XTAL or not (which is what Luca's patch was about). should we use compatible flag in such case? i'm not sure it sounds right. anyway, i'd really prefer, if possible, starting with the wl18xx bindings, and defer the wl12xx complications to later on. (i also need to find some wl12xx card in order to actually test the patches once i'll have them...) we do have to make sure these wl18xx bindings are future-compatible with the wl12xx ones, but i think the current bindings are pretty much standard (and are actually a subset of the bindings needed by wl12xx), so it should be fine. Well how about add just the parsing of the clock and assume it's always WL12XX_REFCLOCK_38 for now as that's the only thing we're currently using. Then we can add a property or compatible value if using something else as needed. Regards, Tony -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 3/3] ARM: dts: igep00x0: add wl18xx bindings
On Wed, Mar 11, 2015 at 3:21 PM, Javier Martinez Canillas jav...@dowhile0.org wrote: On Wed, Mar 11, 2015 at 2:17 PM, Arnd Bergmann a...@arndb.de wrote: On Wednesday 11 March 2015 14:07:11 Javier Martinez Canillas wrote: Right now it seems that all boards in mainline with a WiLink6 part are using internal clocks. So as a first step I think that adding an optional refclock-frequency and tcxoclock-frequency properties should be enough. It would be good if the driver supports getting the refclock and tcxoclock from an external provider in case a board gets these from external clocks but that can be done as a followup if there are boards in the future using that design. But please bear in mind that I'm not familiar with the clock handling in WiLink6 since the WiLink8 part used in the IGEP boards does not need these clocks and I only looked at Luciano's previous patches and the WiLink today driver today. So it would be good if Eliad can double check my assumptions to see if those are correct. sounds right. that's what i know as well. Sounds good. I'd also be fine with not implementing the case for external clocks in the code until we need (and can test) it, but I think it should be specified in the binding from the start. Agreed. great. so i'll implement the internal clocks case only. thanks, Eliad. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 3/3] ARM: dts: igep00x0: add wl18xx bindings
On Wednesday 11 March 2015 02:00:59 Javier Martinez Canillas wrote: Hello Tony, On Tue, Mar 10, 2015 at 6:35 PM, Tony Lindgren t...@atomide.com wrote: we do have to make sure these wl18xx bindings are future-compatible with the wl12xx ones, but i think the current bindings are pretty much standard (and are actually a subset of the bindings needed by wl12xx), so it should be fine. Well how about add just the parsing of the clock and assume it's always WL12XX_REFCLOCK_38 for now as that's the only thing we're currently using. Then we can add a property or compatible value if using something else as needed. What do you mean by parsing here? IIUC there isn't a clock driver for these clocks and are setup directly in the drivers/net/wireless/ti/wl12xx/main.c driver. So you can't make the WLAN chip dev node a consumer of these clocks by adding a phandle to a clock provider and clock specifiers since there isn't a provider to be referenced in the DT. Or did I misunderstand? As I understand it, the clock signal is provided by an external oscillator, which we can easily model in DT, and then you call clk_get_rate on that. If there is no external clock provider for this chip and the clocks are provided by the device itself, then all we need is a clock-frequency property in the device node. Arnd -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 3/3] ARM: dts: igep00x0: add wl18xx bindings
On Wednesday 11 March 2015 14:07:11 Javier Martinez Canillas wrote: Right now it seems that all boards in mainline with a WiLink6 part are using internal clocks. So as a first step I think that adding an optional refclock-frequency and tcxoclock-frequency properties should be enough. It would be good if the driver supports getting the refclock and tcxoclock from an external provider in case a board gets these from external clocks but that can be done as a followup if there are boards in the future using that design. But please bear in mind that I'm not familiar with the clock handling in WiLink6 since the WiLink8 part used in the IGEP boards does not need these clocks and I only looked at Luciano's previous patches and the WiLink today driver today. So it would be good if Eliad can double check my assumptions to see if those are correct. Sounds good. I'd also be fine with not implementing the case for external clocks in the code until we need (and can test) it, but I think it should be specified in the binding from the start. Arnd -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 3/3] ARM: dts: igep00x0: add wl18xx bindings
* Eliad Peller el...@wizery.com [150311 06:39]: On Wed, Mar 11, 2015 at 3:21 PM, Javier Martinez Canillas jav...@dowhile0.org wrote: On Wed, Mar 11, 2015 at 2:17 PM, Arnd Bergmann a...@arndb.de wrote: On Wednesday 11 March 2015 14:07:11 Javier Martinez Canillas wrote: Right now it seems that all boards in mainline with a WiLink6 part are using internal clocks. So as a first step I think that adding an optional refclock-frequency and tcxoclock-frequency properties should be enough. It would be good if the driver supports getting the refclock and tcxoclock from an external provider in case a board gets these from external clocks but that can be done as a followup if there are boards in the future using that design. But please bear in mind that I'm not familiar with the clock handling in WiLink6 since the WiLink8 part used in the IGEP boards does not need these clocks and I only looked at Luciano's previous patches and the WiLink today driver today. So it would be good if Eliad can double check my assumptions to see if those are correct. sounds right. that's what i know as well. Sounds good. I'd also be fine with not implementing the case for external clocks in the code until we need (and can test) it, but I think it should be specified in the binding from the start. Agreed. great. so i'll implement the internal clocks case only. OK great sounds good to me also. Regards, Tony -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 3/3] ARM: dts: igep00x0: add wl18xx bindings
On Tuesday 10 March 2015 16:31:33 Eliad Peller wrote: On Tue, Mar 10, 2015 at 4:11 PM, Arnd Bergmann a...@arndb.de wrote: On Tuesday 10 March 2015 13:00:19 Eliad Peller wrote: On Tue, Mar 10, 2015 at 12:49 AM, Tony Lindgren t...@atomide.com wrote: I was expecting you to remove all calls to legacy_init_wl12xx from this file, including the ones for wl12xx aside from the wl18xx ones you removed, but if that's enough to clean out the platform_data handling from the wlcore driver, it's good enough as a start. not sure i'm following - can you elaborate? i'll summarize the way i see it. please correct me if i'm wrong. both wl18xx and wl12xx use the platform data to get the irq number. wl12xx (only) also needs some additional clock definitions to be passed. there's currently some issue with specifying some the of clock sources, so i preferred starting only with (the simpler) wl18xx bindings. for platforms with wl18xx, we can remove the pdata-quirk, as all the data (i.e. irq) can be passed by the new DT bindings. however, for platforms with wl12xx, we still need to pass the clock definitions (along with the irq), so we have to keep legacy_init_wl12xx for the time being (and that's also why we have to currently keep the platform_data handling in the wlcore driver) do you have something else in mind? I think what Arnd is saying is we've now removed all the wl12xx using legacy platforms, so all of them can boot with just data from dts. Right, that was my idea. I don't think that's the case (unless i'm missing something). e.g. there's still pdata-quirk for compulab,omap3-sbc-t3730 which initializes wl12xx device. This one is just like the igep0030, as Tony was saying: the board boots from device tree already, so now that we have a binding for it, we can remove the wl12xx_set_platform_data() for it. i think the wl12xx_set_platform_data() name created some confusion - it is used to pass platform data for both wl12xx and wl18xx devices. (this confusion is all around the wlcore driver as well, due to the code evolution) the binding i added is for wl18xx only (there is no wl12xx binding yet). the remaining boards, AFAICT, have wl12xx (rather than wl18xx) cards. so i don't see how we can remove these wl12xx_set_platform_data() calls before we have wl12xx bindings in-place as well. What is missing for that binding then? I keep getting confused here, but I thought that they share the implementation that looks at the platform data. Arnd -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 3/3] ARM: dts: igep00x0: add wl18xx bindings
On Monday 09 March 2015 23:03:30 Eliad Peller wrote:
On Mon, Mar 9, 2015 at 9:50 PM, Arnd Bergmann a...@arndb.de wrote:
On Monday 09 March 2015 17:36:42 Eliad Peller wrote:
--- a/arch/arm/boot/dts/omap3-igep0030-rev-g.dts
+++ b/arch/arm/boot/dts/omap3-igep0030-rev-g.dts
@@ -64,4 +64,13 @@
vmmc-supply = lbep5clwmc_wlen;
bus-width = 4;
non-removable;
+
+ #address-cells = 1;
+ #size-cells = 0;
+ wlcore: wlcore@2 {
+ compatible = ti,wl1835;
+ reg = 2;
+ interrupt-parent = gpio5;
+ interrupts = 8 IRQ_TYPE_NONE;
+ };
Why IRQ_TYPE_NONE?
i simply mirrored the current board file (which only sets the irq number).
The irq type is set in this chunk of code from wlcore_nvs_cb:
if (wl-platform_quirks WL12XX_PLATFORM_QUIRK_EDGE_IRQ) {
irqflags = IRQF_TRIGGER_RISING;
hardirq_fn = wlcore_hardirq;
} else {
irqflags = IRQF_TRIGGER_HIGH | IRQF_ONESHOT;
}
This means you would replace the platform_quirks with setting the
correct irq type.
Arnd
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 3/3] ARM: dts: igep00x0: add wl18xx bindings
* Arnd Bergmann a...@arndb.de [150310 08:48]: On Tuesday 10 March 2015 07:28:05 Tony Lindgren wrote: Oops I forgot about the omap3-sbc-t3730, so yes we have to keep the platform data a little bit longer. But nothing stopping us moving all the other ones to use a proper device tree based configuration. For all I can tell, the t3730 board file does not support wl12xx at the moment, only the dts file does. Hmm strange, it seems to configure the wlan_rst pin. Maybe there are variants with different WLAN module. But yeah, seems to be unused. That still leaves configuring all the dts users of the pdata-quirks.c legacy_init_wl12xx() with proper dts before removing it: $ git grep legacy_init_wl12xx pdata-quirks.c pdata-quirks.c:static void __init __used legacy_init_wl12xx(unsigned ref_clock, pdata-quirks.c:static inline void legacy_init_wl12xx(unsigned ref_clock, pdata-quirks.c: legacy_init_wl12xx(WL12XX_REFCLOCK_38, 0, 136); pdata-quirks.c: legacy_init_wl12xx(0, 0, 177); pdata-quirks.c: legacy_init_wl12xx(0, 0, 136); pdata-quirks.c: legacy_init_wl12xx(WL12XX_REFCLOCK_38, 0, 149); pdata-quirks.c: legacy_init_wl12xx(WL12XX_REFCLOCK_26, 0, 162); pdata-quirks.c: legacy_init_wl12xx(WL12XX_REFCLOCK_38, 0, 145); pdata-quirks.c: legacy_init_wl12xx(WL12XX_REFCLOCK_26, pdata-quirks.c: legacy_init_wl12xx(WL12XX_REFCLOCK_38, 0, 53); pdata-quirks.c: legacy_init_wl12xx(WL12XX_REFCLOCK_38, 0, 41); pdata-quirks.c: legacy_init_wl12xx(WL12XX_REFCLOCK_38, 0, 31); If we do what Sekhar suggested and drop wl12xx support from the DA850 EVM board file, we can fix all wlcore users. I guess Sekhar knows the da850 evm status the best. So up to you guys if wl12xx is not being used on legacy da850. Regards, Tony -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 3/3] ARM: dts: igep00x0: add wl18xx bindings
Hello Arnd, On Wed, Mar 11, 2015 at 10:53 AM, Arnd Bergmann a...@arndb.de wrote: On Wednesday 11 March 2015 02:00:59 Javier Martinez Canillas wrote: What do you mean by parsing here? IIUC there isn't a clock driver for these clocks and are setup directly in the drivers/net/wireless/ti/wl12xx/main.c driver. So you can't make the WLAN chip dev node a consumer of these clocks by adding a phandle to a clock provider and clock specifiers since there isn't a provider to be referenced in the DT. Or did I misunderstand? As I understand it, the clock signal is provided by an external oscillator, According to [0], it seems the chip can be connected to both external oscillators or internal clocks provided by the chip itself. which we can easily model in DT, and then you call clk_get_rate on that. Right, my point wast that this can be done only if the external oscillator have a proper clock driver / provider which I don't think is the case here. Most of this stuff predates the common clock framework. Or at least Luciano Coelho had a patch on his series to make the wilink driver a clock provider itself by registering the refclock and tcxoclock clocks [0]. Luciano also had patches for: * Adding the clock provider dev node in the DTS [1] * Have a table to map the clock rate with the FW configuration values [2] * Getting the clock from DT and the rate as you said to configure the firmware accordingly [3] I think that patch [0] should not be needed since for external clocks, the IP providing the clocks should have its own clock driver and for internal clocks, a property should be used instead as you said. If there is no external clock provider for this chip and the clocks are provided by the device itself, then all we need is a clock-frequency property in the device node. Agreed, IIUC Luciano wanted to expose the internal clocks by registering in the common clock framework but if those clocks are not really accessible from outside the wlan chip, then I also think that a device node property should be used instead. Arnd -- Best regards, Javier [0]: https://lists.ozlabs.org/pipermail/devicetree-discuss/2013-July/037139.html [1]: http://lists.infradead.org/pipermail/linux-arm-kernel/2013-July/187794.html [2]: http://lists.infradead.org/pipermail/linux-arm-kernel/2013-July/181594.html [3]: http://lists.infradead.org/pipermail/linux-arm-kernel/2013-July/181591.html -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 3/3] ARM: dts: igep00x0: add wl18xx bindings
Hello Arnd, On Wed, Mar 11, 2015 at 1:40 PM, Arnd Bergmann a...@arndb.de wrote: On Wednesday 11 March 2015 12:34:03 Javier Martinez Canillas wrote: Hello Arnd, On Wed, Mar 11, 2015 at 10:53 AM, Arnd Bergmann a...@arndb.de wrote: On Wednesday 11 March 2015 02:00:59 Javier Martinez Canillas wrote: What do you mean by parsing here? IIUC there isn't a clock driver for these clocks and are setup directly in the drivers/net/wireless/ti/wl12xx/main.c driver. So you can't make the WLAN chip dev node a consumer of these clocks by adding a phandle to a clock provider and clock specifiers since there isn't a provider to be referenced in the DT. Or did I misunderstand? As I understand it, the clock signal is provided by an external oscillator, According to [0], it seems the chip can be connected to both external oscillators or internal clocks provided by the chip itself. I see, that part wasn't clear to me. Yeah, it was not clear to me either before reading Luciano's commit message. [snip] If there is no external clock provider for this chip and the clocks are provided by the device itself, then all we need is a clock-frequency property in the device node. Agreed, IIUC Luciano wanted to expose the internal clocks by registering in the common clock framework but if those clocks are not really accessible from outside the wlan chip, then I also think that a device node property should be used instead. If I understand this right, that measn we can easily distinguish between an external XTAL clock and the internal clock by they way they are described in the DT: for the internal clock, we just provide a clock-frequency property, while the external clock would be referenced through a clocks property. That's my understanding as well. Right now it seems that all boards in mainline with a WiLink6 part are using internal clocks. So as a first step I think that adding an optional refclock-frequency and tcxoclock-frequency properties should be enough. It would be good if the driver supports getting the refclock and tcxoclock from an external provider in case a board gets these from external clocks but that can be done as a followup if there are boards in the future using that design. But please bear in mind that I'm not familiar with the clock handling in WiLink6 since the WiLink8 part used in the IGEP boards does not need these clocks and I only looked at Luciano's previous patches and the WiLink today driver today. So it would be good if Eliad can double check my assumptions to see if those are correct. Best regards, Javier -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 3/3] ARM: dts: igep00x0: add wl18xx bindings
On Wed, Mar 11, 2015 at 2:17 PM, Arnd Bergmann a...@arndb.de wrote: On Wednesday 11 March 2015 14:07:11 Javier Martinez Canillas wrote: Right now it seems that all boards in mainline with a WiLink6 part are using internal clocks. So as a first step I think that adding an optional refclock-frequency and tcxoclock-frequency properties should be enough. It would be good if the driver supports getting the refclock and tcxoclock from an external provider in case a board gets these from external clocks but that can be done as a followup if there are boards in the future using that design. But please bear in mind that I'm not familiar with the clock handling in WiLink6 since the WiLink8 part used in the IGEP boards does not need these clocks and I only looked at Luciano's previous patches and the WiLink today driver today. So it would be good if Eliad can double check my assumptions to see if those are correct. Sounds good. I'd also be fine with not implementing the case for external clocks in the code until we need (and can test) it, but I think it should be specified in the binding from the start. Agreed. Best regards, Javier -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 3/3] ARM: dts: igep00x0: add wl18xx bindings
On Wednesday 11 March 2015 14:12:54 Eliad Peller wrote:
On Wed, Mar 11, 2015 at 1:34 PM, Javier Martinez Canillas
jav...@dowhile0.org wrote:
I think that patch [0] should not be needed since for external clocks,
the IP providing the clocks should have its own clock driver and for
internal clocks, a property should be used instead as you said.
If there is no external clock provider for this chip and the clocks
are provided by the device itself, then all we need is a clock-frequency
property in the device node.
Agreed, IIUC Luciano wanted to expose the internal clocks by
registering in the common clock framework but if those clocks are not
really accessible from outside the wlan chip, then I also think that a
device node property should be used instead.
how should i describe multiple clock-frequency properties (there are 2
relevant clocks) in this case?
does something like the following makes sense?
wlcore: wlcore@2 {
...
refclock: refclock {
compatible = fixed-clock;
#clock-cells = 0;
clock-frequency = 3840;
};
}
I would put that clock node on the top level of the DT, as you are
describing an external clock input here, but other than that, it looks
good.
I would do the binding in a way that mandates either a clocks
reference to an external clock provider in case of a XTAL or
a clock-frequency property. In the first case, you can use
the clock-names property to identify the ref and txco
clock inputs, in the second case we could either decide
to have a single property with two cells, or have named
properties like
wlcore@2 {
interrupts = ... ;
tcxo-clock-frequency = 3840;
ref-clock-frequency = 1920;
}
I don't know which combinations are possible here. I did notice
that most of the references in the board hacks use '0' as the
tcxo clock value, which happens to be the same as
WL12XX_TCXOCLOCK_19_2, but could also meant that no tcxo clock
is used.
Arnd
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 3/3] ARM: dts: igep00x0: add wl18xx bindings
On Wednesday 11 March 2015 12:34:03 Javier Martinez Canillas wrote: Hello Arnd, On Wed, Mar 11, 2015 at 10:53 AM, Arnd Bergmann a...@arndb.de wrote: On Wednesday 11 March 2015 02:00:59 Javier Martinez Canillas wrote: What do you mean by parsing here? IIUC there isn't a clock driver for these clocks and are setup directly in the drivers/net/wireless/ti/wl12xx/main.c driver. So you can't make the WLAN chip dev node a consumer of these clocks by adding a phandle to a clock provider and clock specifiers since there isn't a provider to be referenced in the DT. Or did I misunderstand? As I understand it, the clock signal is provided by an external oscillator, According to [0], it seems the chip can be connected to both external oscillators or internal clocks provided by the chip itself. I see, that part wasn't clear to me. which we can easily model in DT, and then you call clk_get_rate on that. Right, my point wast that this can be done only if the external oscillator have a proper clock driver / provider which I don't think is the case here. Most of this stuff predates the common clock framework. Or at least Luciano Coelho had a patch on his series to make the wilink driver a clock provider itself by registering the refclock and tcxoclock clocks [0]. I guess we should only do this if the clocks from the wilink device might be used by some other device. Luciano also had patches for: * Adding the clock provider dev node in the DTS [1] * Have a table to map the clock rate with the FW configuration values [2] * Getting the clock from DT and the rate as you said to configure the firmware accordingly [3] I think that patch [0] should not be needed since for external clocks, the IP providing the clocks should have its own clock driver and for internal clocks, a property should be used instead as you said. Right. If there is no external clock provider for this chip and the clocks are provided by the device itself, then all we need is a clock-frequency property in the device node. Agreed, IIUC Luciano wanted to expose the internal clocks by registering in the common clock framework but if those clocks are not really accessible from outside the wlan chip, then I also think that a device node property should be used instead. If I understand this right, that measn we can easily distinguish between an external XTAL clock and the internal clock by they way they are described in the DT: for the internal clock, we just provide a clock-frequency property, while the external clock would be referenced through a clocks property. Arnd -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 3/3] ARM: dts: igep00x0: add wl18xx bindings
hi Javier,
On Wed, Mar 11, 2015 at 2:28 AM, Javier Martinez Canillas
jav...@dowhile0.org wrote:
Hello Eliad,
On Mon, Mar 9, 2015 at 4:36 PM, Eliad Peller el...@wizery.com wrote:
Add wl18xx (wilink8) bindings to omap3-igep0030-rev-g and
omap3-igep0020-rev-f dts files, instead of defining the
platform data through the pdata-quirks.
The patch was compile-tested only.
You should look at MAINTAINERS or use ./scripts/get_maintainer.pl to
know who should be cc'ed. Specially for this kind of patches where you
are not able to test on the actual platform. Added Enric to cc as well
since he also maintains the IGEP boards an has access to the hw too.
right. sorry about that.
i assumed adding the relevant mailing lists should be enough, but i
guess i should have added the relevant maintainers as well.
arch/arm/boot/dts/omap3-igep0020-rev-f.dts | 9 +
arch/arm/boot/dts/omap3-igep0030-rev-g.dts | 9 +
arch/arm/mach-omap2/pdata-quirks.c | 2 --
3 files changed, 18 insertions(+), 2 deletions(-)
diff --git a/arch/arm/boot/dts/omap3-igep0020-rev-f.dts
b/arch/arm/boot/dts/omap3-igep0020-rev-f.dts
index cc8bd0c..8e5b44e 100644
--- a/arch/arm/boot/dts/omap3-igep0020-rev-f.dts
+++ b/arch/arm/boot/dts/omap3-igep0020-rev-f.dts
@@ -42,4 +42,13 @@
vmmc-supply = lbep5clwmc_wlen;
bus-width = 4;
non-removable;
+
+ #address-cells = 1;
+ #size-cells = 0;
+ wlcore: wlcore@2 {
+ compatible = ti,wl1835;
+ reg = 2;
+ interrupt-parent = gpio6;
+ interrupts = 17 IRQ_TYPE_NONE;
As Arnd said, it seems this should be IRQ_TYPE_LEVEL_HIGH to match
what the platform code is currently doing.
will be fixed.
thanks,
Eliad.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 3/3] ARM: dts: igep00x0: add wl18xx bindings
hi Javier, On Wed, Mar 11, 2015 at 3:19 AM, Javier Martinez Canillas jav...@dowhile0.org wrote: Hello Eliad, On Wed, Mar 11, 2015 at 1:28 AM, Javier Martinez Canillas jav...@dowhile0.org wrote: On Mon, Mar 9, 2015 at 4:36 PM, Eliad Peller el...@wizery.com wrote: --- a/arch/arm/boot/dts/omap3-igep0020-rev-f.dts +++ b/arch/arm/boot/dts/omap3-igep0020-rev-f.dts @@ -42,4 +42,13 @@ vmmc-supply = lbep5clwmc_wlen; Something I forgot to mention is that this vmmc-supply is a DT hack since the WLAN SDIO chip needs a WL_EN signal to be asserted as a part of the chip power sequencing. But there wasn't a DT binding to describe that so most boards use the same hack which is to define a fake fixed-regulator with an enable GPIO just to toggle that pin. But now the MMC subsystem has support for power sequence providers and the pwrseq-simple driver (Documentation/devicetree/bindings/mmc/mmc-pwrseq-simple.txt) has support for reset GPIOs so you may want to change that as well. interesting. i wasn't aware of it. but that's for a different time i guess :) thanks, Eliad. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 3/3] ARM: dts: igep00x0: add wl18xx bindings
Hello Eliad, On Wed, Mar 11, 2015 at 12:59 PM, Eliad Peller el...@wizery.com wrote: On Wed, Mar 11, 2015 at 3:19 AM, Javier Martinez Canillas jav...@dowhile0.org wrote: But there wasn't a DT binding to describe that so most boards use the same hack which is to define a fake fixed-regulator with an enable GPIO just to toggle that pin. But now the MMC subsystem has support for power sequence providers and the pwrseq-simple driver (Documentation/devicetree/bindings/mmc/mmc-pwrseq-simple.txt) has support for reset GPIOs so you may want to change that as well. interesting. i wasn't aware of it. but that's for a different time i guess :) Indeed, I've on my TODO anyways so I can post a patch on top of your series :-) thanks, Eliad. Best regards, Javier -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 3/3] ARM: dts: igep00x0: add wl18xx bindings
On Wed, Mar 11, 2015 at 1:34 PM, Javier Martinez Canillas
jav...@dowhile0.org wrote:
I think that patch [0] should not be needed since for external clocks,
the IP providing the clocks should have its own clock driver and for
internal clocks, a property should be used instead as you said.
If there is no external clock provider for this chip and the clocks
are provided by the device itself, then all we need is a clock-frequency
property in the device node.
Agreed, IIUC Luciano wanted to expose the internal clocks by
registering in the common clock framework but if those clocks are not
really accessible from outside the wlan chip, then I also think that a
device node property should be used instead.
how should i describe multiple clock-frequency properties (there are 2
relevant clocks) in this case?
does something like the following makes sense?
wlcore: wlcore@2 {
...
refclock: refclock {
compatible = fixed-clock;
#clock-cells = 0;
clock-frequency = 3840;
};
}
Eliad.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 3/3] ARM: dts: igep00x0: add wl18xx bindings
On Tue, Mar 10, 2015 at 12:49 AM, Tony Lindgren t...@atomide.com wrote:
* Eliad Peller el...@wizery.com [150309 14:03]:
On Mon, Mar 9, 2015 at 9:50 PM, Arnd Bergmann a...@arndb.de wrote:
On Monday 09 March 2015 17:36:42 Eliad Peller wrote:
--- a/arch/arm/boot/dts/omap3-igep0030-rev-g.dts
+++ b/arch/arm/boot/dts/omap3-igep0030-rev-g.dts
@@ -64,4 +64,13 @@
vmmc-supply = lbep5clwmc_wlen;
bus-width = 4;
non-removable;
+
+ #address-cells = 1;
+ #size-cells = 0;
+ wlcore: wlcore@2 {
+ compatible = ti,wl1835;
+ reg = 2;
+ interrupt-parent = gpio5;
+ interrupts = 8 IRQ_TYPE_NONE;
+ };
Why IRQ_TYPE_NONE?
i simply mirrored the current board file (which only sets the irq number).
I was expecting you to remove all calls to legacy_init_wl12xx from this
file,
including the ones for wl12xx aside from the wl18xx ones you removed, but
if that's enough to clean out the platform_data handling from the wlcore
driver, it's good enough as a start.
not sure i'm following - can you elaborate?
i'll summarize the way i see it. please correct me if i'm wrong.
both wl18xx and wl12xx use the platform data to get the irq number.
wl12xx (only) also needs some additional clock definitions to be
passed. there's currently some issue with specifying some the of clock
sources, so i preferred starting only with (the simpler) wl18xx
bindings.
for platforms with wl18xx, we can remove the pdata-quirk, as all the
data (i.e. irq) can be passed by the new DT bindings.
however, for platforms with wl12xx, we still need to pass the clock
definitions (along with the irq), so we have to keep
legacy_init_wl12xx for the time being (and that's also why we have to
currently keep the platform_data handling in the wlcore driver)
do you have something else in mind?
I think what Arnd is saying is we've now removed all the wl12xx using
legacy platforms, so all of them can boot with just data from dts.
I don't think that's the case (unless i'm missing something).
e.g. there's still pdata-quirk for compulab,omap3-sbc-t3730 which
initializes wl12xx device.
Eliad.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 3/3] ARM: dts: igep00x0: add wl18xx bindings
* Arnd Bergmann a...@arndb.de [150310 07:11]:
On Tuesday 10 March 2015 13:00:19 Eliad Peller wrote:
On Tue, Mar 10, 2015 at 12:49 AM, Tony Lindgren t...@atomide.com wrote:
* Eliad Peller el...@wizery.com [150309 14:03]:
On Mon, Mar 9, 2015 at 9:50 PM, Arnd Bergmann a...@arndb.de wrote:
On Monday 09 March 2015 17:36:42 Eliad Peller wrote:
--- a/arch/arm/boot/dts/omap3-igep0030-rev-g.dts
+++ b/arch/arm/boot/dts/omap3-igep0030-rev-g.dts
@@ -64,4 +64,13 @@
vmmc-supply = lbep5clwmc_wlen;
bus-width = 4;
non-removable;
+
+ #address-cells = 1;
+ #size-cells = 0;
+ wlcore: wlcore@2 {
+ compatible = ti,wl1835;
+ reg = 2;
+ interrupt-parent = gpio5;
+ interrupts = 8 IRQ_TYPE_NONE;
+ };
Why IRQ_TYPE_NONE?
i simply mirrored the current board file (which only sets the irq
number).
I was expecting you to remove all calls to legacy_init_wl12xx from
this file,
including the ones for wl12xx aside from the wl18xx ones you removed,
but
if that's enough to clean out the platform_data handling from the
wlcore
driver, it's good enough as a start.
not sure i'm following - can you elaborate?
i'll summarize the way i see it. please correct me if i'm wrong.
both wl18xx and wl12xx use the platform data to get the irq number.
wl12xx (only) also needs some additional clock definitions to be
passed. there's currently some issue with specifying some the of clock
sources, so i preferred starting only with (the simpler) wl18xx
bindings.
for platforms with wl18xx, we can remove the pdata-quirk, as all the
data (i.e. irq) can be passed by the new DT bindings.
however, for platforms with wl12xx, we still need to pass the clock
definitions (along with the irq), so we have to keep
legacy_init_wl12xx for the time being (and that's also why we have to
currently keep the platform_data handling in the wlcore driver)
do you have something else in mind?
I think what Arnd is saying is we've now removed all the wl12xx using
legacy platforms, so all of them can boot with just data from dts.
Right, that was my idea.
I don't think that's the case (unless i'm missing something).
e.g. there's still pdata-quirk for compulab,omap3-sbc-t3730 which
initializes wl12xx device.
This one is just like the igep0030, as Tony was saying: the board
boots from device tree already, so now that we have a binding for
it, we can remove the wl12xx_set_platform_data() for it.
Unfortunately, there is still one machine that uses a board file
in combination with this wl12xx: da850-evm calls
wl12xx_set_platform_data() from a legacy board file without DT.
We have DT support for this file as well, but if I remember correctly,
it is somewhat incomplete and we don't want to remove the file
yet. Adding Sekhar and Kevin to Cc here for confirmation.
Oops I forgot about the omap3-sbc-t3730, so yes we have to keep the
platform data a little bit longer. But nothing stopping us moving
all the other ones to use a proper device tree based configuration.
Regards,
Tony
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 3/3] ARM: dts: igep00x0: add wl18xx bindings
On Tue, Mar 10, 2015 at 4:11 PM, Arnd Bergmann a...@arndb.de wrote: On Tuesday 10 March 2015 13:00:19 Eliad Peller wrote: On Tue, Mar 10, 2015 at 12:49 AM, Tony Lindgren t...@atomide.com wrote: I was expecting you to remove all calls to legacy_init_wl12xx from this file, including the ones for wl12xx aside from the wl18xx ones you removed, but if that's enough to clean out the platform_data handling from the wlcore driver, it's good enough as a start. not sure i'm following - can you elaborate? i'll summarize the way i see it. please correct me if i'm wrong. both wl18xx and wl12xx use the platform data to get the irq number. wl12xx (only) also needs some additional clock definitions to be passed. there's currently some issue with specifying some the of clock sources, so i preferred starting only with (the simpler) wl18xx bindings. for platforms with wl18xx, we can remove the pdata-quirk, as all the data (i.e. irq) can be passed by the new DT bindings. however, for platforms with wl12xx, we still need to pass the clock definitions (along with the irq), so we have to keep legacy_init_wl12xx for the time being (and that's also why we have to currently keep the platform_data handling in the wlcore driver) do you have something else in mind? I think what Arnd is saying is we've now removed all the wl12xx using legacy platforms, so all of them can boot with just data from dts. Right, that was my idea. I don't think that's the case (unless i'm missing something). e.g. there's still pdata-quirk for compulab,omap3-sbc-t3730 which initializes wl12xx device. This one is just like the igep0030, as Tony was saying: the board boots from device tree already, so now that we have a binding for it, we can remove the wl12xx_set_platform_data() for it. i think the wl12xx_set_platform_data() name created some confusion - it is used to pass platform data for both wl12xx and wl18xx devices. (this confusion is all around the wlcore driver as well, due to the code evolution) the binding i added is for wl18xx only (there is no wl12xx binding yet). the remaining boards, AFAICT, have wl12xx (rather than wl18xx) cards. so i don't see how we can remove these wl12xx_set_platform_data() calls before we have wl12xx bindings in-place as well. Unfortunately, there is still one machine that uses a board file in combination with this wl12xx: da850-evm calls wl12xx_set_platform_data() from a legacy board file without DT. ditto. Eliad. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 3/3] ARM: dts: igep00x0: add wl18xx bindings
On Tuesday 10 March 2015 07:41 PM, Arnd Bergmann wrote:
On Tuesday 10 March 2015 13:00:19 Eliad Peller wrote:
On Tue, Mar 10, 2015 at 12:49 AM, Tony Lindgren t...@atomide.com wrote:
* Eliad Peller el...@wizery.com [150309 14:03]:
On Mon, Mar 9, 2015 at 9:50 PM, Arnd Bergmann a...@arndb.de wrote:
On Monday 09 March 2015 17:36:42 Eliad Peller wrote:
--- a/arch/arm/boot/dts/omap3-igep0030-rev-g.dts
+++ b/arch/arm/boot/dts/omap3-igep0030-rev-g.dts
@@ -64,4 +64,13 @@
vmmc-supply = lbep5clwmc_wlen;
bus-width = 4;
non-removable;
+
+ #address-cells = 1;
+ #size-cells = 0;
+ wlcore: wlcore@2 {
+ compatible = ti,wl1835;
+ reg = 2;
+ interrupt-parent = gpio5;
+ interrupts = 8 IRQ_TYPE_NONE;
+ };
Why IRQ_TYPE_NONE?
i simply mirrored the current board file (which only sets the irq number).
I was expecting you to remove all calls to legacy_init_wl12xx from this
file,
including the ones for wl12xx aside from the wl18xx ones you removed, but
if that's enough to clean out the platform_data handling from the wlcore
driver, it's good enough as a start.
not sure i'm following - can you elaborate?
i'll summarize the way i see it. please correct me if i'm wrong.
both wl18xx and wl12xx use the platform data to get the irq number.
wl12xx (only) also needs some additional clock definitions to be
passed. there's currently some issue with specifying some the of clock
sources, so i preferred starting only with (the simpler) wl18xx
bindings.
for platforms with wl18xx, we can remove the pdata-quirk, as all the
data (i.e. irq) can be passed by the new DT bindings.
however, for platforms with wl12xx, we still need to pass the clock
definitions (along with the irq), so we have to keep
legacy_init_wl12xx for the time being (and that's also why we have to
currently keep the platform_data handling in the wlcore driver)
do you have something else in mind?
I think what Arnd is saying is we've now removed all the wl12xx using
legacy platforms, so all of them can boot with just data from dts.
Right, that was my idea.
I don't think that's the case (unless i'm missing something).
e.g. there's still pdata-quirk for compulab,omap3-sbc-t3730 which
initializes wl12xx device.
This one is just like the igep0030, as Tony was saying: the board
boots from device tree already, so now that we have a binding for
it, we can remove the wl12xx_set_platform_data() for it.
Unfortunately, there is still one machine that uses a board file
in combination with this wl12xx: da850-evm calls
wl12xx_set_platform_data() from a legacy board file without DT.
We have DT support for this file as well, but if I remember correctly,
it is somewhat incomplete and we don't want to remove the file
yet. Adding Sekhar and Kevin to Cc here for confirmation.
Thats true, we do not want to drop legacy booting for DA850 EVM yet.
But if its coming in the way of code consolidation, I do not mind
dropping WLAN support for that board. Anyone who really needs it can add
support to the DA850 EVM DT file.
Thanks,
Sekhar
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 3/3] ARM: dts: igep00x0: add wl18xx bindings
On Tuesday 10 March 2015 13:00:19 Eliad Peller wrote:
On Tue, Mar 10, 2015 at 12:49 AM, Tony Lindgren t...@atomide.com wrote:
* Eliad Peller el...@wizery.com [150309 14:03]:
On Mon, Mar 9, 2015 at 9:50 PM, Arnd Bergmann a...@arndb.de wrote:
On Monday 09 March 2015 17:36:42 Eliad Peller wrote:
--- a/arch/arm/boot/dts/omap3-igep0030-rev-g.dts
+++ b/arch/arm/boot/dts/omap3-igep0030-rev-g.dts
@@ -64,4 +64,13 @@
vmmc-supply = lbep5clwmc_wlen;
bus-width = 4;
non-removable;
+
+ #address-cells = 1;
+ #size-cells = 0;
+ wlcore: wlcore@2 {
+ compatible = ti,wl1835;
+ reg = 2;
+ interrupt-parent = gpio5;
+ interrupts = 8 IRQ_TYPE_NONE;
+ };
Why IRQ_TYPE_NONE?
i simply mirrored the current board file (which only sets the irq number).
I was expecting you to remove all calls to legacy_init_wl12xx from this
file,
including the ones for wl12xx aside from the wl18xx ones you removed, but
if that's enough to clean out the platform_data handling from the wlcore
driver, it's good enough as a start.
not sure i'm following - can you elaborate?
i'll summarize the way i see it. please correct me if i'm wrong.
both wl18xx and wl12xx use the platform data to get the irq number.
wl12xx (only) also needs some additional clock definitions to be
passed. there's currently some issue with specifying some the of clock
sources, so i preferred starting only with (the simpler) wl18xx
bindings.
for platforms with wl18xx, we can remove the pdata-quirk, as all the
data (i.e. irq) can be passed by the new DT bindings.
however, for platforms with wl12xx, we still need to pass the clock
definitions (along with the irq), so we have to keep
legacy_init_wl12xx for the time being (and that's also why we have to
currently keep the platform_data handling in the wlcore driver)
do you have something else in mind?
I think what Arnd is saying is we've now removed all the wl12xx using
legacy platforms, so all of them can boot with just data from dts.
Right, that was my idea.
I don't think that's the case (unless i'm missing something).
e.g. there's still pdata-quirk for compulab,omap3-sbc-t3730 which
initializes wl12xx device.
This one is just like the igep0030, as Tony was saying: the board
boots from device tree already, so now that we have a binding for
it, we can remove the wl12xx_set_platform_data() for it.
Unfortunately, there is still one machine that uses a board file
in combination with this wl12xx: da850-evm calls
wl12xx_set_platform_data() from a legacy board file without DT.
We have DT support for this file as well, but if I remember correctly,
it is somewhat incomplete and we don't want to remove the file
yet. Adding Sekhar and Kevin to Cc here for confirmation.
Arnd
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 3/3] ARM: dts: igep00x0: add wl18xx bindings
On Tue, Mar 10, 2015 at 6:18 PM, Tony Lindgren t...@atomide.com wrote: * Eliad Peller el...@wizery.com [150310 09:11]: On Tue, Mar 10, 2015 at 5:52 PM, Arnd Bergmann a...@arndb.de wrote: On Tuesday 10 March 2015 16:31:33 Eliad Peller wrote: On Tue, Mar 10, 2015 at 4:11 PM, Arnd Bergmann a...@arndb.de wrote: On Tuesday 10 March 2015 13:00:19 Eliad Peller wrote: On Tue, Mar 10, 2015 at 12:49 AM, Tony Lindgren t...@atomide.com wrote: I was expecting you to remove all calls to legacy_init_wl12xx from this file, including the ones for wl12xx aside from the wl18xx ones you removed, but if that's enough to clean out the platform_data handling from the wlcore driver, it's good enough as a start. not sure i'm following - can you elaborate? i'll summarize the way i see it. please correct me if i'm wrong. both wl18xx and wl12xx use the platform data to get the irq number. wl12xx (only) also needs some additional clock definitions to be passed. there's currently some issue with specifying some the of clock sources, so i preferred starting only with (the simpler) wl18xx bindings. for platforms with wl18xx, we can remove the pdata-quirk, as all the data (i.e. irq) can be passed by the new DT bindings. however, for platforms with wl12xx, we still need to pass the clock definitions (along with the irq), so we have to keep legacy_init_wl12xx for the time being (and that's also why we have to currently keep the platform_data handling in the wlcore driver) do you have something else in mind? I think what Arnd is saying is we've now removed all the wl12xx using legacy platforms, so all of them can boot with just data from dts. Right, that was my idea. I don't think that's the case (unless i'm missing something). e.g. there's still pdata-quirk for compulab,omap3-sbc-t3730 which initializes wl12xx device. This one is just like the igep0030, as Tony was saying: the board boots from device tree already, so now that we have a binding for it, we can remove the wl12xx_set_platform_data() for it. i think the wl12xx_set_platform_data() name created some confusion - it is used to pass platform data for both wl12xx and wl18xx devices. (this confusion is all around the wlcore driver as well, due to the code evolution) the binding i added is for wl18xx only (there is no wl12xx binding yet). the remaining boards, AFAICT, have wl12xx (rather than wl18xx) cards. so i don't see how we can remove these wl12xx_set_platform_data() calls before we have wl12xx bindings in-place as well. What is missing for that binding then? I keep getting confused here, but I thought that they share the implementation that looks at the platform data. they both get the same wl12xx_platform_data struct, but only wl12xx cares about the clocks-related fields. the bindings i added parses only the irq. (Luca tried previously to upstream wl12xx DT support along with the required clock DT changes, but got some rejections, mainly wrt. clock stuff. e.g. http://thread.gmane.org/gmane.linux.kernel/1520752 that's why i preferred starting with easier wl18xx bindings only) I believe we did not have clock bindings back then, now it's simple to get the clock. If it's some internal clock to the wl12xx, then that's a different story, it should be just hidden behind a compatible flag then. i'm really not familiar with the common clock framework, but there still doesn't seem to be a way to determine whether a clock is XTAL or not (which is what Luca's patch was about). should we use compatible flag in such case? i'm not sure it sounds right. anyway, i'd really prefer, if possible, starting with the wl18xx bindings, and defer the wl12xx complications to later on. (i also need to find some wl12xx card in order to actually test the patches once i'll have them...) we do have to make sure these wl18xx bindings are future-compatible with the wl12xx ones, but i think the current bindings are pretty much standard (and are actually a subset of the bindings needed by wl12xx), so it should be fine. Eliad. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 3/3] ARM: dts: igep00x0: add wl18xx bindings
Hello Tony, On Tue, Mar 10, 2015 at 6:35 PM, Tony Lindgren t...@atomide.com wrote: we do have to make sure these wl18xx bindings are future-compatible with the wl12xx ones, but i think the current bindings are pretty much standard (and are actually a subset of the bindings needed by wl12xx), so it should be fine. Well how about add just the parsing of the clock and assume it's always WL12XX_REFCLOCK_38 for now as that's the only thing we're currently using. Then we can add a property or compatible value if using something else as needed. What do you mean by parsing here? IIUC there isn't a clock driver for these clocks and are setup directly in the drivers/net/wireless/ti/wl12xx/main.c driver. So you can't make the WLAN chip dev node a consumer of these clocks by adding a phandle to a clock provider and clock specifiers since there isn't a provider to be referenced in the DT. Or did I misunderstand? Regards, Tony Best regards, Javier -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 3/3] ARM: dts: igep00x0: add wl18xx bindings
Hello Eliad, On Wed, Mar 11, 2015 at 1:28 AM, Javier Martinez Canillas jav...@dowhile0.org wrote: On Mon, Mar 9, 2015 at 4:36 PM, Eliad Peller el...@wizery.com wrote: --- a/arch/arm/boot/dts/omap3-igep0020-rev-f.dts +++ b/arch/arm/boot/dts/omap3-igep0020-rev-f.dts @@ -42,4 +42,13 @@ vmmc-supply = lbep5clwmc_wlen; Something I forgot to mention is that this vmmc-supply is a DT hack since the WLAN SDIO chip needs a WL_EN signal to be asserted as a part of the chip power sequencing. But there wasn't a DT binding to describe that so most boards use the same hack which is to define a fake fixed-regulator with an enable GPIO just to toggle that pin. But now the MMC subsystem has support for power sequence providers and the pwrseq-simple driver (Documentation/devicetree/bindings/mmc/mmc-pwrseq-simple.txt) has support for reset GPIOs so you may want to change that as well. In fact, the pwrseq-simple also has support for an external clock (and can be extended to support more than one) so if the clocks are not internal to the wl12xx, maybe these should be defined in the pwrseq-simple dev node assuming that there is a clock driver for them. Best regards, Javier -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 3/3] ARM: dts: igep00x0: add wl18xx bindings
On Tue, Mar 10, 2015 at 5:52 PM, Arnd Bergmann a...@arndb.de wrote: On Tuesday 10 March 2015 16:31:33 Eliad Peller wrote: On Tue, Mar 10, 2015 at 4:11 PM, Arnd Bergmann a...@arndb.de wrote: On Tuesday 10 March 2015 13:00:19 Eliad Peller wrote: On Tue, Mar 10, 2015 at 12:49 AM, Tony Lindgren t...@atomide.com wrote: I was expecting you to remove all calls to legacy_init_wl12xx from this file, including the ones for wl12xx aside from the wl18xx ones you removed, but if that's enough to clean out the platform_data handling from the wlcore driver, it's good enough as a start. not sure i'm following - can you elaborate? i'll summarize the way i see it. please correct me if i'm wrong. both wl18xx and wl12xx use the platform data to get the irq number. wl12xx (only) also needs some additional clock definitions to be passed. there's currently some issue with specifying some the of clock sources, so i preferred starting only with (the simpler) wl18xx bindings. for platforms with wl18xx, we can remove the pdata-quirk, as all the data (i.e. irq) can be passed by the new DT bindings. however, for platforms with wl12xx, we still need to pass the clock definitions (along with the irq), so we have to keep legacy_init_wl12xx for the time being (and that's also why we have to currently keep the platform_data handling in the wlcore driver) do you have something else in mind? I think what Arnd is saying is we've now removed all the wl12xx using legacy platforms, so all of them can boot with just data from dts. Right, that was my idea. I don't think that's the case (unless i'm missing something). e.g. there's still pdata-quirk for compulab,omap3-sbc-t3730 which initializes wl12xx device. This one is just like the igep0030, as Tony was saying: the board boots from device tree already, so now that we have a binding for it, we can remove the wl12xx_set_platform_data() for it. i think the wl12xx_set_platform_data() name created some confusion - it is used to pass platform data for both wl12xx and wl18xx devices. (this confusion is all around the wlcore driver as well, due to the code evolution) the binding i added is for wl18xx only (there is no wl12xx binding yet). the remaining boards, AFAICT, have wl12xx (rather than wl18xx) cards. so i don't see how we can remove these wl12xx_set_platform_data() calls before we have wl12xx bindings in-place as well. What is missing for that binding then? I keep getting confused here, but I thought that they share the implementation that looks at the platform data. they both get the same wl12xx_platform_data struct, but only wl12xx cares about the clocks-related fields. the bindings i added parses only the irq. (Luca tried previously to upstream wl12xx DT support along with the required clock DT changes, but got some rejections, mainly wrt. clock stuff. e.g. http://thread.gmane.org/gmane.linux.kernel/1520752 that's why i preferred starting with easier wl18xx bindings only) Eliad. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 3/3] ARM: dts: igep00x0: add wl18xx bindings
On Tuesday 10 March 2015 07:28:05 Tony Lindgren wrote: Oops I forgot about the omap3-sbc-t3730, so yes we have to keep the platform data a little bit longer. But nothing stopping us moving all the other ones to use a proper device tree based configuration. For all I can tell, the t3730 board file does not support wl12xx at the moment, only the dts file does. If we do what Sekhar suggested and drop wl12xx support from the DA850 EVM board file, we can fix all wlcore users. Arnd -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 3/3] ARM: dts: igep00x0: add wl18xx bindings
* Eliad Peller el...@wizery.com [150310 09:11]: On Tue, Mar 10, 2015 at 5:52 PM, Arnd Bergmann a...@arndb.de wrote: On Tuesday 10 March 2015 16:31:33 Eliad Peller wrote: On Tue, Mar 10, 2015 at 4:11 PM, Arnd Bergmann a...@arndb.de wrote: On Tuesday 10 March 2015 13:00:19 Eliad Peller wrote: On Tue, Mar 10, 2015 at 12:49 AM, Tony Lindgren t...@atomide.com wrote: I was expecting you to remove all calls to legacy_init_wl12xx from this file, including the ones for wl12xx aside from the wl18xx ones you removed, but if that's enough to clean out the platform_data handling from the wlcore driver, it's good enough as a start. not sure i'm following - can you elaborate? i'll summarize the way i see it. please correct me if i'm wrong. both wl18xx and wl12xx use the platform data to get the irq number. wl12xx (only) also needs some additional clock definitions to be passed. there's currently some issue with specifying some the of clock sources, so i preferred starting only with (the simpler) wl18xx bindings. for platforms with wl18xx, we can remove the pdata-quirk, as all the data (i.e. irq) can be passed by the new DT bindings. however, for platforms with wl12xx, we still need to pass the clock definitions (along with the irq), so we have to keep legacy_init_wl12xx for the time being (and that's also why we have to currently keep the platform_data handling in the wlcore driver) do you have something else in mind? I think what Arnd is saying is we've now removed all the wl12xx using legacy platforms, so all of them can boot with just data from dts. Right, that was my idea. I don't think that's the case (unless i'm missing something). e.g. there's still pdata-quirk for compulab,omap3-sbc-t3730 which initializes wl12xx device. This one is just like the igep0030, as Tony was saying: the board boots from device tree already, so now that we have a binding for it, we can remove the wl12xx_set_platform_data() for it. i think the wl12xx_set_platform_data() name created some confusion - it is used to pass platform data for both wl12xx and wl18xx devices. (this confusion is all around the wlcore driver as well, due to the code evolution) the binding i added is for wl18xx only (there is no wl12xx binding yet). the remaining boards, AFAICT, have wl12xx (rather than wl18xx) cards. so i don't see how we can remove these wl12xx_set_platform_data() calls before we have wl12xx bindings in-place as well. What is missing for that binding then? I keep getting confused here, but I thought that they share the implementation that looks at the platform data. they both get the same wl12xx_platform_data struct, but only wl12xx cares about the clocks-related fields. the bindings i added parses only the irq. (Luca tried previously to upstream wl12xx DT support along with the required clock DT changes, but got some rejections, mainly wrt. clock stuff. e.g. http://thread.gmane.org/gmane.linux.kernel/1520752 that's why i preferred starting with easier wl18xx bindings only) I believe we did not have clock bindings back then, now it's simple to get the clock. If it's some internal clock to the wl12xx, then that's a different story, it should be just hidden behind a compatible flag then. Regards, Tony -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 3/3] ARM: dts: igep00x0: add wl18xx bindings
On Tuesday 10 March 2015 10:35:50 Tony Lindgren wrote:
* Eliad Peller el...@wizery.com [150310 10:01]:
i'm really not familiar with the common clock framework, but there
still doesn't seem to be a way to determine whether a clock is XTAL or
not (which is what Luca's patch was about). should we use compatible
flag in such case? i'm not sure it sounds right.
anyway, i'd really prefer, if possible, starting with the wl18xx
bindings, and defer the wl12xx complications to later on.
(i also need to find some wl12xx card in order to actually test the
patches once i'll have them...)
we do have to make sure these wl18xx bindings are future-compatible
with the wl12xx ones, but i think the current bindings are pretty much
standard (and are actually a subset of the bindings needed by wl12xx),
so it should be fine.
Well how about add just the parsing of the clock and assume it's always
WL12XX_REFCLOCK_38 for now as that's the only thing we're currently
using. Then we can add a property or compatible value if using something
else as needed.
We have two exceptions:
static void __init omap3_zoom_legacy_init(void)
{
legacy_init_wl12xx(WL12XX_REFCLOCK_26, 0, 162);
}
static void __init omap4_sdp_legacy_init(void)
{
legacy_init_wl12xx(WL12XX_REFCLOCK_26,
WL12XX_TCXOCLOCK_26, 53);
}
Where do those clocks come from? If these are always fixed-rate
clocks coming from an external clock provider, using a separate
compatible string in the clock provider would seem reasonable to
me, but we can do that once we have to: none of the machines
we support use an XTAL clock input.
Arnd
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 3/3] ARM: dts: igep00x0: add wl18xx bindings
Hello Eliad,
On Mon, Mar 9, 2015 at 4:36 PM, Eliad Peller el...@wizery.com wrote:
Add wl18xx (wilink8) bindings to omap3-igep0030-rev-g and
omap3-igep0020-rev-f dts files, instead of defining the
platform data through the pdata-quirks.
The patch was compile-tested only.
You should look at MAINTAINERS or use ./scripts/get_maintainer.pl to
know who should be cc'ed. Specially for this kind of patches where you
are not able to test on the actual platform. Added Enric to cc as well
since he also maintains the IGEP boards an has access to the hw too.
Signed-off-by: Eliad Peller el...@wizery.com
---
arch/arm/boot/dts/omap3-igep0020-rev-f.dts | 9 +
arch/arm/boot/dts/omap3-igep0030-rev-g.dts | 9 +
arch/arm/mach-omap2/pdata-quirks.c | 2 --
3 files changed, 18 insertions(+), 2 deletions(-)
diff --git a/arch/arm/boot/dts/omap3-igep0020-rev-f.dts
b/arch/arm/boot/dts/omap3-igep0020-rev-f.dts
index cc8bd0c..8e5b44e 100644
--- a/arch/arm/boot/dts/omap3-igep0020-rev-f.dts
+++ b/arch/arm/boot/dts/omap3-igep0020-rev-f.dts
@@ -42,4 +42,13 @@
vmmc-supply = lbep5clwmc_wlen;
bus-width = 4;
non-removable;
+
+ #address-cells = 1;
+ #size-cells = 0;
+ wlcore: wlcore@2 {
+ compatible = ti,wl1835;
+ reg = 2;
+ interrupt-parent = gpio6;
+ interrupts = 17 IRQ_TYPE_NONE;
As Arnd said, it seems this should be IRQ_TYPE_LEVEL_HIGH to match
what the platform code is currently doing.
Best regards,
Javier
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 3/3] ARM: dts: igep00x0: add wl18xx bindings
Add wl18xx (wilink8) bindings to omap3-igep0030-rev-g and
omap3-igep0020-rev-f dts files, instead of defining the
platform data through the pdata-quirks.
The patch was compile-tested only.
Signed-off-by: Eliad Peller el...@wizery.com
---
arch/arm/boot/dts/omap3-igep0020-rev-f.dts | 9 +
arch/arm/boot/dts/omap3-igep0030-rev-g.dts | 9 +
arch/arm/mach-omap2/pdata-quirks.c | 2 --
3 files changed, 18 insertions(+), 2 deletions(-)
diff --git a/arch/arm/boot/dts/omap3-igep0020-rev-f.dts
b/arch/arm/boot/dts/omap3-igep0020-rev-f.dts
index cc8bd0c..8e5b44e 100644
--- a/arch/arm/boot/dts/omap3-igep0020-rev-f.dts
+++ b/arch/arm/boot/dts/omap3-igep0020-rev-f.dts
@@ -42,4 +42,13 @@
vmmc-supply = lbep5clwmc_wlen;
bus-width = 4;
non-removable;
+
+ #address-cells = 1;
+ #size-cells = 0;
+ wlcore: wlcore@2 {
+ compatible = ti,wl1835;
+ reg = 2;
+ interrupt-parent = gpio6;
+ interrupts = 17 IRQ_TYPE_NONE;
+ };
};
diff --git a/arch/arm/boot/dts/omap3-igep0030-rev-g.dts
b/arch/arm/boot/dts/omap3-igep0030-rev-g.dts
index 9326b28..02c4eec 100644
--- a/arch/arm/boot/dts/omap3-igep0030-rev-g.dts
+++ b/arch/arm/boot/dts/omap3-igep0030-rev-g.dts
@@ -64,4 +64,13 @@
vmmc-supply = lbep5clwmc_wlen;
bus-width = 4;
non-removable;
+
+ #address-cells = 1;
+ #size-cells = 0;
+ wlcore: wlcore@2 {
+ compatible = ti,wl1835;
+ reg = 2;
+ interrupt-parent = gpio5;
+ interrupts = 8 IRQ_TYPE_NONE;
+ };
};
diff --git a/arch/arm/mach-omap2/pdata-quirks.c
b/arch/arm/mach-omap2/pdata-quirks.c
index 190fa43..abc8a20 100644
--- a/arch/arm/mach-omap2/pdata-quirks.c
+++ b/arch/arm/mach-omap2/pdata-quirks.c
@@ -159,14 +159,12 @@ static struct platform_device btwilink_device = {
static void __init omap3_igep0020_rev_f_legacy_init(void)
{
- legacy_init_wl12xx(0, 0, 177);
platform_device_register(wl18xx_device);
platform_device_register(btwilink_device);
}
static void __init omap3_igep0030_rev_g_legacy_init(void)
{
- legacy_init_wl12xx(0, 0, 136);
platform_device_register(wl18xx_device);
platform_device_register(btwilink_device);
}
--
1.8.5.2.229.g4448466.dirty
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 3/3] ARM: dts: igep00x0: add wl18xx bindings
On Mon, Mar 9, 2015 at 9:50 PM, Arnd Bergmann a...@arndb.de wrote:
On Monday 09 March 2015 17:36:42 Eliad Peller wrote:
--- a/arch/arm/boot/dts/omap3-igep0030-rev-g.dts
+++ b/arch/arm/boot/dts/omap3-igep0030-rev-g.dts
@@ -64,4 +64,13 @@
vmmc-supply = lbep5clwmc_wlen;
bus-width = 4;
non-removable;
+
+ #address-cells = 1;
+ #size-cells = 0;
+ wlcore: wlcore@2 {
+ compatible = ti,wl1835;
+ reg = 2;
+ interrupt-parent = gpio5;
+ interrupts = 8 IRQ_TYPE_NONE;
+ };
Why IRQ_TYPE_NONE?
i simply mirrored the current board file (which only sets the irq number).
I was expecting you to remove all calls to legacy_init_wl12xx from this file,
including the ones for wl12xx aside from the wl18xx ones you removed, but
if that's enough to clean out the platform_data handling from the wlcore
driver, it's good enough as a start.
not sure i'm following - can you elaborate?
i'll summarize the way i see it. please correct me if i'm wrong.
both wl18xx and wl12xx use the platform data to get the irq number.
wl12xx (only) also needs some additional clock definitions to be
passed. there's currently some issue with specifying some the of clock
sources, so i preferred starting only with (the simpler) wl18xx
bindings.
for platforms with wl18xx, we can remove the pdata-quirk, as all the
data (i.e. irq) can be passed by the new DT bindings.
however, for platforms with wl12xx, we still need to pass the clock
definitions (along with the irq), so we have to keep
legacy_init_wl12xx for the time being (and that's also why we have to
currently keep the platform_data handling in the wlcore driver)
do you have something else in mind?
Eliad.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 3/3] ARM: dts: igep00x0: add wl18xx bindings
On Monday 09 March 2015 17:36:42 Eliad Peller wrote:
--- a/arch/arm/boot/dts/omap3-igep0030-rev-g.dts
+++ b/arch/arm/boot/dts/omap3-igep0030-rev-g.dts
@@ -64,4 +64,13 @@
vmmc-supply = lbep5clwmc_wlen;
bus-width = 4;
non-removable;
+
+ #address-cells = 1;
+ #size-cells = 0;
+ wlcore: wlcore@2 {
+ compatible = ti,wl1835;
+ reg = 2;
+ interrupt-parent = gpio5;
+ interrupts = 8 IRQ_TYPE_NONE;
+ };
Why IRQ_TYPE_NONE?
I was expecting you to remove all calls to legacy_init_wl12xx from this file,
including the ones for wl12xx aside from the wl18xx ones you removed, but
if that's enough to clean out the platform_data handling from the wlcore
driver, it's good enough as a start.
Arnd
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 3/3] ARM: dts: igep00x0: add wl18xx bindings
* Eliad Peller el...@wizery.com [150309 14:03]:
On Mon, Mar 9, 2015 at 9:50 PM, Arnd Bergmann a...@arndb.de wrote:
On Monday 09 March 2015 17:36:42 Eliad Peller wrote:
--- a/arch/arm/boot/dts/omap3-igep0030-rev-g.dts
+++ b/arch/arm/boot/dts/omap3-igep0030-rev-g.dts
@@ -64,4 +64,13 @@
vmmc-supply = lbep5clwmc_wlen;
bus-width = 4;
non-removable;
+
+ #address-cells = 1;
+ #size-cells = 0;
+ wlcore: wlcore@2 {
+ compatible = ti,wl1835;
+ reg = 2;
+ interrupt-parent = gpio5;
+ interrupts = 8 IRQ_TYPE_NONE;
+ };
Why IRQ_TYPE_NONE?
i simply mirrored the current board file (which only sets the irq number).
I was expecting you to remove all calls to legacy_init_wl12xx from this
file,
including the ones for wl12xx aside from the wl18xx ones you removed, but
if that's enough to clean out the platform_data handling from the wlcore
driver, it's good enough as a start.
not sure i'm following - can you elaborate?
i'll summarize the way i see it. please correct me if i'm wrong.
both wl18xx and wl12xx use the platform data to get the irq number.
wl12xx (only) also needs some additional clock definitions to be
passed. there's currently some issue with specifying some the of clock
sources, so i preferred starting only with (the simpler) wl18xx
bindings.
for platforms with wl18xx, we can remove the pdata-quirk, as all the
data (i.e. irq) can be passed by the new DT bindings.
however, for platforms with wl12xx, we still need to pass the clock
definitions (along with the irq), so we have to keep
legacy_init_wl12xx for the time being (and that's also why we have to
currently keep the platform_data handling in the wlcore driver)
do you have something else in mind?
I think what Arnd is saying is we've now removed all the wl12xx using
legacy platforms, so all of them can boot with just data from dts.
Regards,
Tony
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
