Re: [PATCH v5 3/3] ARM: dts: igep00x0: add wl18xx bindings

Arnd Bergmann
2015-03-10T14:11:36Z

On Tuesday 10 March 2015 13:00:19 Eliad Peller wrote:
> On Tue, Mar 10, 2015 at 12:49 AM, Tony Lindgren <tony@atomide.com> wrote:
> > * Eliad Peller <eliad@wizery.com> [150309 14:03]:
> >> On Mon, Mar 9, 2015 at 9:50 PM, Arnd Bergmann <arnd@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