| Message ID | 20190607172958.20745-1-erosca@de.adit-jv.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Kalle Valo |
| Headers | show |
| Series | wlcore/wl18xx: Add invert-irq OF property for physically inverted IRQ | expand |
Eugeniu Rosca <erosca@de.adit-jv.com> writes: > The wl1837mod datasheet [1] says about the WL_IRQ pin: > > ---8<--- > SDIO available, interrupt out. Active high. [..] > Set to rising edge (active high) on powerup. > ---8<--- > > That's the reason of seeing the interrupt configured as: > - IRQ_TYPE_EDGE_RISING on HiKey 960/970 > - IRQ_TYPE_LEVEL_HIGH on a number of i.MX6 platforms > > We assert that all those platforms have the WL_IRQ pin connected > to the SoC _directly_ (confirmed on HiKey 970 [2]). > > That's not the case for R-Car Kingfisher extension target, which carries > a WL1837MODGIMOCT IC. There is an SN74LV1T04DBVR inverter present > between the WLAN_IRQ pin of the WL18* chip and the SoC, effectively > reversing the requirement quoted from [1]. IOW, in Kingfisher DTS > configuration we would need to use IRQ_TYPE_EDGE_FALLING or > IRQ_TYPE_LEVEL_LOW. > > Unfortunately, v4.2-rc1 commit bd763482c82ea2 ("wl18xx: wlan_irq: > support platform dependent interrupt types") made a special case out > of these interrupt types. After this commit, it is impossible to provide > an IRQ configuration via DTS which would describe an inverter present > between the WL18* chip and the SoC, generating the need for workarounds > like [3]. > > Create a boolean OF property, called "invert-irq" to specify that > the WLAN_IRQ pin of WL18* is connected to the SoC via an inverter. > > This solution has been successfully tested on R-Car H3ULCB-KF-M06 using > the DTS configuration [4] combined with the "invert-irq" property. > > [1] http://www.ti.com/lit/ds/symlink/wl1837mod.pdf > [2] https://www.96boards.org/documentation/consumer/hikey/hikey970/hardware-docs/ > [3] https://github.com/CogentEmbedded/meta-rcar/blob/289fbd4f8354/meta-rcar-gen3-adas/recipes-kernel/linux/linux-renesas/0024-wl18xx-do-not-invert-IRQ-on-WLxxxx-side.patch > [4] https://patchwork.kernel.org/patch/10895879/ > ("arm64: dts: ulcb-kf: Add support for TI WL1837") > > Signed-off-by: Eugeniu Rosca <erosca@de.adit-jv.com> Tony&Eyal, do you agree with this?
Hi, * Kalle Valo <kvalo@codeaurora.org> [190610 07:01]: > Eugeniu Rosca <erosca@de.adit-jv.com> writes: > > > The wl1837mod datasheet [1] says about the WL_IRQ pin: > > > > ---8<--- > > SDIO available, interrupt out. Active high. [..] > > Set to rising edge (active high) on powerup. > > ---8<--- > > > > That's the reason of seeing the interrupt configured as: > > - IRQ_TYPE_EDGE_RISING on HiKey 960/970 > > - IRQ_TYPE_LEVEL_HIGH on a number of i.MX6 platforms > > > > We assert that all those platforms have the WL_IRQ pin connected > > to the SoC _directly_ (confirmed on HiKey 970 [2]). > > > > That's not the case for R-Car Kingfisher extension target, which carries > > a WL1837MODGIMOCT IC. There is an SN74LV1T04DBVR inverter present > > between the WLAN_IRQ pin of the WL18* chip and the SoC, effectively > > reversing the requirement quoted from [1]. IOW, in Kingfisher DTS > > configuration we would need to use IRQ_TYPE_EDGE_FALLING or > > IRQ_TYPE_LEVEL_LOW. > > > > Unfortunately, v4.2-rc1 commit bd763482c82ea2 ("wl18xx: wlan_irq: > > support platform dependent interrupt types") made a special case out > > of these interrupt types. After this commit, it is impossible to provide > > an IRQ configuration via DTS which would describe an inverter present > > between the WL18* chip and the SoC, generating the need for workarounds > > like [3]. > > > > Create a boolean OF property, called "invert-irq" to specify that > > the WLAN_IRQ pin of WL18* is connected to the SoC via an inverter. > > > > This solution has been successfully tested on R-Car H3ULCB-KF-M06 using > > the DTS configuration [4] combined with the "invert-irq" property. > > > > [1] http://www.ti.com/lit/ds/symlink/wl1837mod.pdf > > [2] https://www.96boards.org/documentation/consumer/hikey/hikey970/hardware-docs/ > > [3] https://github.com/CogentEmbedded/meta-rcar/blob/289fbd4f8354/meta-rcar-gen3-adas/recipes-kernel/linux/linux-renesas/0024-wl18xx-do-not-invert-IRQ-on-WLxxxx-side.patch > > [4] https://patchwork.kernel.org/patch/10895879/ > > ("arm64: dts: ulcb-kf: Add support for TI WL1837") > > > > Signed-off-by: Eugeniu Rosca <erosca@de.adit-jv.com> > > Tony&Eyal, do you agree with this? Yeah if there's some hardware between the WLAN device and the SoC inverting the interrupt, I don't think we have clear a way to deal with it short of setting up a separate irqchip that does the translation. But in some cases we also do not want to invert the interrupt, so I think this property should take IRQ_TYPE_EDGE_RISING and IRQ_TYPE_EDGE_RISING values to override the setting for the WLAN end of the hardware? Let's wait a bit longer for comments from Eyal too. Regards, Tony
CC irqchip Original thread at https://lore.kernel.org/lkml/20190607172958.20745-1-erosca@de.adit-jv.com/ On Mon, Jun 10, 2019 at 10:30 AM Tony Lindgren <tony@atomide.com> wrote: > * Kalle Valo <kvalo@codeaurora.org> [190610 07:01]: > > Eugeniu Rosca <erosca@de.adit-jv.com> writes: > > > > > The wl1837mod datasheet [1] says about the WL_IRQ pin: > > > > > > ---8<--- > > > SDIO available, interrupt out. Active high. [..] > > > Set to rising edge (active high) on powerup. > > > ---8<--- > > > > > > That's the reason of seeing the interrupt configured as: > > > - IRQ_TYPE_EDGE_RISING on HiKey 960/970 > > > - IRQ_TYPE_LEVEL_HIGH on a number of i.MX6 platforms > > > > > > We assert that all those platforms have the WL_IRQ pin connected > > > to the SoC _directly_ (confirmed on HiKey 970 [2]). > > > > > > That's not the case for R-Car Kingfisher extension target, which carries > > > a WL1837MODGIMOCT IC. There is an SN74LV1T04DBVR inverter present > > > between the WLAN_IRQ pin of the WL18* chip and the SoC, effectively > > > reversing the requirement quoted from [1]. IOW, in Kingfisher DTS > > > configuration we would need to use IRQ_TYPE_EDGE_FALLING or > > > IRQ_TYPE_LEVEL_LOW. > > > > > > Unfortunately, v4.2-rc1 commit bd763482c82ea2 ("wl18xx: wlan_irq: > > > support platform dependent interrupt types") made a special case out > > > of these interrupt types. After this commit, it is impossible to provide > > > an IRQ configuration via DTS which would describe an inverter present > > > between the WL18* chip and the SoC, generating the need for workarounds > > > like [3]. > > > > > > Create a boolean OF property, called "invert-irq" to specify that > > > the WLAN_IRQ pin of WL18* is connected to the SoC via an inverter. > > > > > > This solution has been successfully tested on R-Car H3ULCB-KF-M06 using > > > the DTS configuration [4] combined with the "invert-irq" property. > > > > > > [1] http://www.ti.com/lit/ds/symlink/wl1837mod.pdf > > > [2] https://www.96boards.org/documentation/consumer/hikey/hikey970/hardware-docs/ > > > [3] https://github.com/CogentEmbedded/meta-rcar/blob/289fbd4f8354/meta-rcar-gen3-adas/recipes-kernel/linux/linux-renesas/0024-wl18xx-do-not-invert-IRQ-on-WLxxxx-side.patch > > > [4] https://patchwork.kernel.org/patch/10895879/ > > > ("arm64: dts: ulcb-kf: Add support for TI WL1837") > > > > > > Signed-off-by: Eugeniu Rosca <erosca@de.adit-jv.com> > > > > Tony&Eyal, do you agree with this? > > Yeah if there's some hardware between the WLAN device and the SoC > inverting the interrupt, I don't think we have clear a way to deal > with it short of setting up a separate irqchip that does the > translation. Yeah, inverting the interrupt type in DT works only for simple devices, that don't need configuration. A simple irqchip driver that just inverts the type sounds like a good solution to me. Does something like that already exists? > But in some cases we also do not want to invert the interrupt, so > I think this property should take IRQ_TYPE_EDGE_RISING and > IRQ_TYPE_EDGE_RISING values to override the setting for > the WLAN end of the hardware? > > Let's wait a bit longer for comments from Eyal too. Gr{oetje,eeting}s, Geert
On 11/06/2019 09:45, Geert Uytterhoeven wrote: > CC irqchip > > Original thread at > https://lore.kernel.org/lkml/20190607172958.20745-1-erosca@de.adit-jv.com/ > > On Mon, Jun 10, 2019 at 10:30 AM Tony Lindgren <tony@atomide.com> wrote: >> * Kalle Valo <kvalo@codeaurora.org> [190610 07:01]: >>> Eugeniu Rosca <erosca@de.adit-jv.com> writes: >>> >>>> The wl1837mod datasheet [1] says about the WL_IRQ pin: >>>> >>>> ---8<--- >>>> SDIO available, interrupt out. Active high. [..] >>>> Set to rising edge (active high) on powerup. >>>> ---8<--- >>>> >>>> That's the reason of seeing the interrupt configured as: >>>> - IRQ_TYPE_EDGE_RISING on HiKey 960/970 >>>> - IRQ_TYPE_LEVEL_HIGH on a number of i.MX6 platforms >>>> >>>> We assert that all those platforms have the WL_IRQ pin connected >>>> to the SoC _directly_ (confirmed on HiKey 970 [2]). >>>> >>>> That's not the case for R-Car Kingfisher extension target, which carries >>>> a WL1837MODGIMOCT IC. There is an SN74LV1T04DBVR inverter present >>>> between the WLAN_IRQ pin of the WL18* chip and the SoC, effectively >>>> reversing the requirement quoted from [1]. IOW, in Kingfisher DTS >>>> configuration we would need to use IRQ_TYPE_EDGE_FALLING or >>>> IRQ_TYPE_LEVEL_LOW. >>>> >>>> Unfortunately, v4.2-rc1 commit bd763482c82ea2 ("wl18xx: wlan_irq: >>>> support platform dependent interrupt types") made a special case out >>>> of these interrupt types. After this commit, it is impossible to provide >>>> an IRQ configuration via DTS which would describe an inverter present >>>> between the WL18* chip and the SoC, generating the need for workarounds >>>> like [3]. >>>> >>>> Create a boolean OF property, called "invert-irq" to specify that >>>> the WLAN_IRQ pin of WL18* is connected to the SoC via an inverter. >>>> >>>> This solution has been successfully tested on R-Car H3ULCB-KF-M06 using >>>> the DTS configuration [4] combined with the "invert-irq" property. >>>> >>>> [1] http://www.ti.com/lit/ds/symlink/wl1837mod.pdf >>>> [2] https://www.96boards.org/documentation/consumer/hikey/hikey970/hardware-docs/ >>>> [3] https://github.com/CogentEmbedded/meta-rcar/blob/289fbd4f8354/meta-rcar-gen3-adas/recipes-kernel/linux/linux-renesas/0024-wl18xx-do-not-invert-IRQ-on-WLxxxx-side.patch >>>> [4] https://patchwork.kernel.org/patch/10895879/ >>>> ("arm64: dts: ulcb-kf: Add support for TI WL1837") >>>> >>>> Signed-off-by: Eugeniu Rosca <erosca@de.adit-jv.com> >>> >>> Tony&Eyal, do you agree with this? >> >> Yeah if there's some hardware between the WLAN device and the SoC >> inverting the interrupt, I don't think we have clear a way to deal >> with it short of setting up a separate irqchip that does the >> translation. > > Yeah, inverting the interrupt type in DT works only for simple devices, > that don't need configuration. > A simple irqchip driver that just inverts the type sounds like a good > solution to me. Does something like that already exists? We already have plenty of that in the tree, the canonical example probably being drivers/irqchip/irq-mtk-sysirq.c. It should be pretty easy to turn this driver into something more generic. Thanks, M.
Hi, cc: Linus Walleij On Tue, Jun 11, 2019 at 10:00:41AM +0100, Marc Zyngier wrote: > On 11/06/2019 09:45, Geert Uytterhoeven wrote: > > CC irqchip > > > > Original thread at > > https://lore.kernel.org/lkml/20190607172958.20745-1-erosca@de.adit-jv.com/ > > > > On Mon, Jun 10, 2019 at 10:30 AM Tony Lindgren <tony@atomide.com> wrote: > >> * Kalle Valo <kvalo@codeaurora.org> [190610 07:01]: > >>> Eugeniu Rosca <erosca@de.adit-jv.com> writes: > >>> > >>>> The wl1837mod datasheet [1] says about the WL_IRQ pin: > >>>> > >>>> ---8<--- > >>>> SDIO available, interrupt out. Active high. [..] > >>>> Set to rising edge (active high) on powerup. > >>>> ---8<--- > >>>> > >>>> That's the reason of seeing the interrupt configured as: > >>>> - IRQ_TYPE_EDGE_RISING on HiKey 960/970 > >>>> - IRQ_TYPE_LEVEL_HIGH on a number of i.MX6 platforms > >>>> > >>>> We assert that all those platforms have the WL_IRQ pin connected > >>>> to the SoC _directly_ (confirmed on HiKey 970 [2]). > >>>> > >>>> That's not the case for R-Car Kingfisher extension target, which carries > >>>> a WL1837MODGIMOCT IC. There is an SN74LV1T04DBVR inverter present > >>>> between the WLAN_IRQ pin of the WL18* chip and the SoC, effectively > >>>> reversing the requirement quoted from [1]. IOW, in Kingfisher DTS > >>>> configuration we would need to use IRQ_TYPE_EDGE_FALLING or > >>>> IRQ_TYPE_LEVEL_LOW. > >>>> > >>>> Unfortunately, v4.2-rc1 commit bd763482c82ea2 ("wl18xx: wlan_irq: > >>>> support platform dependent interrupt types") made a special case out > >>>> of these interrupt types. After this commit, it is impossible to provide > >>>> an IRQ configuration via DTS which would describe an inverter present > >>>> between the WL18* chip and the SoC, generating the need for workarounds > >>>> like [3]. > >>>> > >>>> Create a boolean OF property, called "invert-irq" to specify that > >>>> the WLAN_IRQ pin of WL18* is connected to the SoC via an inverter. > >>>> > >>>> This solution has been successfully tested on R-Car H3ULCB-KF-M06 using > >>>> the DTS configuration [4] combined with the "invert-irq" property. > >>>> > >>>> [1] http://www.ti.com/lit/ds/symlink/wl1837mod.pdf > >>>> [2] https://www.96boards.org/documentation/consumer/hikey/hikey970/hardware-docs/ > >>>> [3] https://github.com/CogentEmbedded/meta-rcar/blob/289fbd4f8354/meta-rcar-gen3-adas/recipes-kernel/linux/linux-renesas/0024-wl18xx-do-not-invert-IRQ-on-WLxxxx-side.patch > >>>> [4] https://patchwork.kernel.org/patch/10895879/ > >>>> ("arm64: dts: ulcb-kf: Add support for TI WL1837") > >>>> > >>>> Signed-off-by: Eugeniu Rosca <erosca@de.adit-jv.com> > >>> > >>> Tony&Eyal, do you agree with this? > >> > >> Yeah if there's some hardware between the WLAN device and the SoC > >> inverting the interrupt, I don't think we have clear a way to deal > >> with it short of setting up a separate irqchip that does the > >> translation. > > > > Yeah, inverting the interrupt type in DT works only for simple devices, > > that don't need configuration. > > A simple irqchip driver that just inverts the type sounds like a good > > solution to me. Does something like that already exists? > > We already have plenty of that in the tree, the canonical example > probably being drivers/irqchip/irq-mtk-sysirq.c. It should be pretty > easy to turn this driver into something more generic. I don't think drivers/irqchip/irq-mtk-sysirq.c can serve the use-case/purpose of this patch. The MTK driver seems to be dealing with the polarity inversion of on-SoC interrupts which are routed to GiC, whereas in this patch we are talking about an off-chip interrupt wired to R-Car GPIO controller. It looks to me that the nice DTS sketch shared by Linus Walleij in [5] might come closer to the concept proposed by Geert? FWIW, the infrastructure/implementation to make this possible is still not ready. One question to the wlcore/wl18xx maintainers: Why exactly do you give freedom to users to set the interrupt as LEVEL_LOW/EDGE_FALLING [6]? Apparently, this: - complicates the wl18xx driver, thus increasing the chance for bugs - is not supposed to reflect any HW differences between boards using LEVEL_LOW/EDGE_FALLING and the boards using LEVEL_HIGH/EDGE_RISING - doesn't bring any obvious advantage to the users, who are expected to sense the same behavior regardless of the IRQ type set in DTS - prevent the users to set IRQ type to LEVEL_LOW/EDGE_FALLING when there is an inverter present between WL_IRQ and SoC - seems to be not used almost at all, as 99% of mainline DTS set the IRQ type to the canonical/NLCP LEVEL_HIGH/EDGE_RISING [5] https://patchwork.ozlabs.org/patch/1095690/#2167076 ("[V1,1/2] gpio: make it possible to set active-state on GPIO lines") --------------------8<------------------- gpio0: gpio { compatible = "foo,chip"; gpio-controller; (...) }; inv0: inverter { compatible = "inverter"; gpio-controller; gpios = <&gpio0 0 GPIO_ACTIVE_HIGH>; }; consumer { compatible = "bar"; gpios = <&inv0 0 GPIO_ACTIVE_HIGH>; }; --------------------8<------------------- [6] bd763482c82ea2 ("wl18xx: wlan_irq: support platform dependent interrupt types")
Hi Marc, Thanks for your comment. On Wed, Jun 12, 2019 at 11:17:10AM +0100, Marc Zyngier wrote: > Eugeniu Rosca <erosca@de.adit-jv.com> wrote: > > On Tue, Jun 11, 2019 at 10:00:41AM +0100, Marc Zyngier wrote: [..] > > > We already have plenty of that in the tree, the canonical example > > > probably being drivers/irqchip/irq-mtk-sysirq.c. It should be pretty > > > easy to turn this driver into something more generic. > > > > I don't think drivers/irqchip/irq-mtk-sysirq.c can serve the > > use-case/purpose of this patch. The MTK driver seems to be dealing with > > the polarity inversion of on-SoC interrupts which are routed to GiC, > > whereas in this patch we are talking about an off-chip interrupt > > wired to R-Car GPIO controller. > > And how different is that? The location of the interrupt source is > pretty irrelevant here. The main difference which I sense is that a driver like irq-mtk-sysirq mostly (if not exclusively) deals with internal kernel implementation detail (tuned via DT) whilst adding an inverter for GPIO IRQs raises a whole bunch of new questions (e.g. how to arbitrate between kernel-space and user-space IRQ polarity configuration?). > The point is that there is already a general > scheme to deal with these "signal altering widgets", and that we > should try to reuse at least the concept, if not the code. Since Harish Jenny K N might be working on a new driver doing GPIO IRQ inversion, I have CC-ed him as well to avoid any overlapping work. > > > It looks to me that the nice DTS sketch shared by Linus Walleij in [5] > > might come closer to the concept proposed by Geert? FWIW, the > > infrastructure/implementation to make this possible is still not > > ready. > > Which looks like what I'm suggesting. Then we are on the same page. Thanks. > > M. > > -- > Jazz is not dead, it just smells funny.
On 12/06/19 8:36 PM, Eugeniu Rosca wrote: > Hi Marc, > > Thanks for your comment. > > On Wed, Jun 12, 2019 at 11:17:10AM +0100, Marc Zyngier wrote: >> Eugeniu Rosca <erosca@de.adit-jv.com> wrote: >>> On Tue, Jun 11, 2019 at 10:00:41AM +0100, Marc Zyngier wrote: > [..] >>>> We already have plenty of that in the tree, the canonical example >>>> probably being drivers/irqchip/irq-mtk-sysirq.c. It should be pretty >>>> easy to turn this driver into something more generic. >>> I don't think drivers/irqchip/irq-mtk-sysirq.c can serve the >>> use-case/purpose of this patch. The MTK driver seems to be dealing with >>> the polarity inversion of on-SoC interrupts which are routed to GiC, >>> whereas in this patch we are talking about an off-chip interrupt >>> wired to R-Car GPIO controller. >> And how different is that? The location of the interrupt source is >> pretty irrelevant here. > The main difference which I sense is that a driver like irq-mtk-sysirq > mostly (if not exclusively) deals with internal kernel implementation > detail (tuned via DT) whilst adding an inverter for GPIO IRQs raises > a whole bunch of new questions (e.g. how to arbitrate between > kernel-space and user-space IRQ polarity configuration?). > >> The point is that there is already a general >> scheme to deal with these "signal altering widgets", and that we >> should try to reuse at least the concept, if not the code. > Since Harish Jenny K N might be working on a new driver doing GPIO IRQ > inversion, I have CC-ed him as well to avoid any overlapping work. Sorry I am not completely aware of the background discussion. But here is the link to my proposal for new consumer driver to provide a new virtual gpio controller to configure the polarity of the gpio pins used by the userspace. https://www.spinics.net/lists/linux-gpio/msg39681.html > >>> It looks to me that the nice DTS sketch shared by Linus Walleij in [5] >>> might come closer to the concept proposed by Geert? FWIW, the >>> infrastructure/implementation to make this possible is still not >>> ready. >> Which looks like what I'm suggesting. > Then we are on the same page. Thanks. > >> M. >> >> -- >> Jazz is not dead, it just smells funny.
Eugeniu Rosca <erosca@de.adit-jv.com> wrote: > The wl1837mod datasheet [1] says about the WL_IRQ pin: > > ---8<--- > SDIO available, interrupt out. Active high. [..] > Set to rising edge (active high) on powerup. > ---8<--- > > That's the reason of seeing the interrupt configured as: > - IRQ_TYPE_EDGE_RISING on HiKey 960/970 > - IRQ_TYPE_LEVEL_HIGH on a number of i.MX6 platforms > > We assert that all those platforms have the WL_IRQ pin connected > to the SoC _directly_ (confirmed on HiKey 970 [2]). > > That's not the case for R-Car Kingfisher extension target, which carries > a WL1837MODGIMOCT IC. There is an SN74LV1T04DBVR inverter present > between the WLAN_IRQ pin of the WL18* chip and the SoC, effectively > reversing the requirement quoted from [1]. IOW, in Kingfisher DTS > configuration we would need to use IRQ_TYPE_EDGE_FALLING or > IRQ_TYPE_LEVEL_LOW. > > Unfortunately, v4.2-rc1 commit bd763482c82ea2 ("wl18xx: wlan_irq: > support platform dependent interrupt types") made a special case out > of these interrupt types. After this commit, it is impossible to provide > an IRQ configuration via DTS which would describe an inverter present > between the WL18* chip and the SoC, generating the need for workarounds > like [3]. > > Create a boolean OF property, called "invert-irq" to specify that > the WLAN_IRQ pin of WL18* is connected to the SoC via an inverter. > > This solution has been successfully tested on R-Car H3ULCB-KF-M06 using > the DTS configuration [4] combined with the "invert-irq" property. > > [1] http://www.ti.com/lit/ds/symlink/wl1837mod.pdf > [2] https://www.96boards.org/documentation/consumer/hikey/hikey970/hardware-docs/ > [3] https://github.com/CogentEmbedded/meta-rcar/blob/289fbd4f8354/meta-rcar-gen3-adas/recipes-kernel/linux/linux-renesas/0024-wl18xx-do-not-invert-IRQ-on-WLxxxx-side.patch > [4] https://patchwork.kernel.org/patch/10895879/ > ("arm64: dts: ulcb-kf: Add support for TI WL1837") > > Signed-off-by: Eugeniu Rosca <erosca@de.adit-jv.com> Based on the discussion I'm dropping this. Please resend once there's a conclusion. Patch set to Changes Requested.
diff --git a/drivers/net/wireless/ti/wl18xx/main.c b/drivers/net/wireless/ti/wl18xx/main.c index 496b9b63cea1..cea91d1aee98 100644 --- a/drivers/net/wireless/ti/wl18xx/main.c +++ b/drivers/net/wireless/ti/wl18xx/main.c @@ -877,6 +877,8 @@ static int wl18xx_pre_boot(struct wl1271 *wl) static int wl18xx_pre_upload(struct wl1271 *wl) { + struct platform_device *pdev = wl->pdev; + struct wlcore_platdev_data *pdata = dev_get_platdata(&pdev->dev); u32 tmp; int ret; u16 irq_invert; @@ -932,6 +934,9 @@ static int wl18xx_pre_upload(struct wl1271 *wl) if (ret < 0) goto out; + if (pdata->invert_irq) + goto out; + ret = irq_get_trigger_type(wl->irq); if ((ret == IRQ_TYPE_LEVEL_LOW) || (ret == IRQ_TYPE_EDGE_FALLING)) { wl1271_info("using inverted interrupt logic: %d", ret); diff --git a/drivers/net/wireless/ti/wlcore/sdio.c b/drivers/net/wireless/ti/wlcore/sdio.c index 4d4b07701149..581f56b0b6a2 100644 --- a/drivers/net/wireless/ti/wlcore/sdio.c +++ b/drivers/net/wireless/ti/wlcore/sdio.c @@ -266,6 +266,8 @@ static int wlcore_probe_of(struct device *dev, int *irq, int *wakeirq, of_property_read_u32(np, "tcxo-clock-frequency", &pdev_data->tcxo_clock_freq); + pdev_data->invert_irq = of_property_read_bool(np, "invert-irq"); + return 0; } #else diff --git a/drivers/net/wireless/ti/wlcore/wlcore_i.h b/drivers/net/wireless/ti/wlcore/wlcore_i.h index 32ec121ccac2..01679f9d7170 100644 --- a/drivers/net/wireless/ti/wlcore/wlcore_i.h +++ b/drivers/net/wireless/ti/wlcore/wlcore_i.h @@ -213,6 +213,10 @@ struct wlcore_platdev_data { u32 ref_clock_freq; /* in Hertz */ u32 tcxo_clock_freq; /* in Hertz, tcxo is always XTAL */ bool pwr_in_suspend; + bool invert_irq; /* specify if there is a physical IRQ inverter + * between WL chip and SoC, like SN74LV1T04DBVR + * in case of R-Car Kingfisher board + */ }; #define MAX_NUM_KEYS 14
The wl1837mod datasheet [1] says about the WL_IRQ pin: ---8<--- SDIO available, interrupt out. Active high. [..] Set to rising edge (active high) on powerup. ---8<--- That's the reason of seeing the interrupt configured as: - IRQ_TYPE_EDGE_RISING on HiKey 960/970 - IRQ_TYPE_LEVEL_HIGH on a number of i.MX6 platforms We assert that all those platforms have the WL_IRQ pin connected to the SoC _directly_ (confirmed on HiKey 970 [2]). That's not the case for R-Car Kingfisher extension target, which carries a WL1837MODGIMOCT IC. There is an SN74LV1T04DBVR inverter present between the WLAN_IRQ pin of the WL18* chip and the SoC, effectively reversing the requirement quoted from [1]. IOW, in Kingfisher DTS configuration we would need to use IRQ_TYPE_EDGE_FALLING or IRQ_TYPE_LEVEL_LOW. Unfortunately, v4.2-rc1 commit bd763482c82ea2 ("wl18xx: wlan_irq: support platform dependent interrupt types") made a special case out of these interrupt types. After this commit, it is impossible to provide an IRQ configuration via DTS which would describe an inverter present between the WL18* chip and the SoC, generating the need for workarounds like [3]. Create a boolean OF property, called "invert-irq" to specify that the WLAN_IRQ pin of WL18* is connected to the SoC via an inverter. This solution has been successfully tested on R-Car H3ULCB-KF-M06 using the DTS configuration [4] combined with the "invert-irq" property. [1] http://www.ti.com/lit/ds/symlink/wl1837mod.pdf [2] https://www.96boards.org/documentation/consumer/hikey/hikey970/hardware-docs/ [3] https://github.com/CogentEmbedded/meta-rcar/blob/289fbd4f8354/meta-rcar-gen3-adas/recipes-kernel/linux/linux-renesas/0024-wl18xx-do-not-invert-IRQ-on-WLxxxx-side.patch [4] https://patchwork.kernel.org/patch/10895879/ ("arm64: dts: ulcb-kf: Add support for TI WL1837") Signed-off-by: Eugeniu Rosca <erosca@de.adit-jv.com> --- drivers/net/wireless/ti/wl18xx/main.c | 5 +++++ drivers/net/wireless/ti/wlcore/sdio.c | 2 ++ drivers/net/wireless/ti/wlcore/wlcore_i.h | 4 ++++ 3 files changed, 11 insertions(+)