Return-path: Received: from mail-qa0-f42.google.com ([209.85.216.42]:34123 "EHLO mail-qa0-f42.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751525AbbASIo7 (ORCPT ); Mon, 19 Jan 2015 03:44:59 -0500 Received: by mail-qa0-f42.google.com with SMTP id dc16so22919430qab.1 for ; Mon, 19 Jan 2015 00:44:59 -0800 (PST) MIME-Version: 1.0 In-Reply-To: References: <20150118051222.32019.32719.stgit@114-36-241-182.dynamic.hinet.net> <20150118051650.32019.49483.stgit@114-36-241-182.dynamic.hinet.net> Date: Mon, 19 Jan 2015 16:44:59 +0800 Message-ID: (sfid-20150119_094512_234155_9ED9D182) Subject: Re: [PATCH 2/2] net wireless wcn36xx adapt wcnss platform to select module by DT From: Andy Green To: Eugene Krasnikov Cc: Kalle Valo , wcn36xx , linux-wireless , netdev Content-Type: text/plain; charset=UTF-8 Sender: linux-wireless-owner@vger.kernel.org List-ID: On 19 January 2015 at 16:34, Eugene Krasnikov wrote: > So how do we insmod wcn36xx_msm with a parameter specifying what type > of hardware do we use? The type of chip is defined in the DT "compatible" which also delivers the resource information. qcom,wcn36xx@0a000000 { compatible = "qcom,wcn3620"; reg = <0x0a000000 0x280000>; reg-names = "wcnss_mmio"; interrupts = <0 145 0 0 146 0>; interrupt-names = "wcnss_wlantx_irq", "wcnss_wlanrx_irq"; ... This bit based on your code can't go in mainline until there's some kind of PIL support. So the only things we can discuss about it for mainline purpose is whether using a platform ops is a good way to interface to the mainline driver. If you're OK with that and you want a module parameter then this can grow a module parameter and prefer to deliver the chip type from that if given, without modifying the platform op interface. But with or without a module parameter this can't be upstreamed right now due to PIL. -Andy > 2015-01-18 5:16 GMT+00:00 Andy Green : >> Simplify the resource handling and use DT to indicate which chip type >> we are dealing with >> >> Signed-off-by: Andy Green >> --- >> drivers/net/wireless/ath/wcn36xx/wcn36xx-msm.c | 101 ++++++++++++------------ >> 1 file changed, 52 insertions(+), 49 deletions(-) >> >> diff --git a/drivers/net/wireless/ath/wcn36xx/wcn36xx-msm.c b/drivers/net/wireless/ath/wcn36xx/wcn36xx-msm.c >> index f6f6c83..c9250e0 100644 >> --- a/drivers/net/wireless/ath/wcn36xx/wcn36xx-msm.c >> +++ b/drivers/net/wireless/ath/wcn36xx/wcn36xx-msm.c >> @@ -42,7 +42,10 @@ struct wcn36xx_msm { >> struct completion smd_compl; >> smd_channel_t *smd_ch; >> struct pinctrl *pinctrl; >> -} wmsm; >> + enum wcn36xx_chip_type chip_type; >> +}; >> + >> +static struct wcn36xx_msm wmsm; >> >> static int wcn36xx_msm_smsm_change_state(u32 clear_mask, u32 set_mask) >> { >> @@ -217,14 +220,47 @@ int wcn36xx_msm_powerup(const struct subsys_desc *desc) >> return 0; >> } >> >> +static const struct of_device_id wcn36xx_msm_match_table[] = { >> + { .compatible = "qcom,wcn3660", .data = (void *)WCN36XX_CHIP_3660 }, >> + { .compatible = "qcom,wcn3680", .data = (void *)WCN36XX_CHIP_3680 }, >> + { .compatible = "qcom,wcn3620", .data = (void *)WCN36XX_CHIP_3620 }, >> + { } >> +}; >> + >> +static int wcn36xx_msm_get_chip_type(void) >> +{ >> + return wmsm.chip_type; >> +} >> + >> +static struct wcn36xx_msm wmsm = { >> + .ctrl_ops = { >> + .open = wcn36xx_msm_smd_open, >> + .close = wcn36xx_msm_smd_close, >> + .tx = wcn36xx_msm_smd_send_and_wait, >> + .get_hw_mac = wcn36xx_msm_get_hw_mac, >> + .smsm_change_state = wcn36xx_msm_smsm_change_state, >> + .get_chip_type = wcn36xx_msm_get_chip_type, >> + }, >> +}; >> + >> static int wcn36xx_msm_probe(struct platform_device *pdev) >> { >> int ret; >> - struct resource *wcnss_memory; >> - struct resource *tx_irq; >> - struct resource *rx_irq; >> + const struct of_device_id *of_id; >> + struct resource *r; >> struct resource res[3]; >> struct pinctrl_state *ps; >> + static const char const *rnames[] = { >> + "wcnss_mmio", "wcnss_wlantx_irq", "wcnss_wlanrx_irq" }; >> + static const int rtype[] = { >> + IORESOURCE_MEM, IORESOURCE_IRQ, IORESOURCE_IRQ }; >> + int n; >> + >> + of_id = of_match_node(wcn36xx_msm_match_table, pdev->dev.of_node); >> + if (!of_id) >> + return -EINVAL; >> + >> + wmsm.chip_type = (enum wcn36xx_chip_type)of_id->data; >> >> wmsm.pinctrl = devm_pinctrl_get(&pdev->dev); >> if (IS_ERR_OR_NULL(wmsm.pinctrl)) >> @@ -240,52 +276,23 @@ static int wcn36xx_msm_probe(struct platform_device *pdev) >> >> if (IS_ERR_OR_NULL(pil)) >> pil = subsystem_get("wcnss"); >> - if (IS_ERR_OR_NULL(pil)) >> - return PTR_ERR(pil); >> + if (IS_ERR_OR_NULL(pil)) >> + return PTR_ERR(pil); >> >> wmsm.core = platform_device_alloc("wcn36xx", -1); >> >> - //dev_err(&pdev->dev, "%s starting\n", __func__); >> - >> - memset(res, 0x00, sizeof(res)); >> - wmsm.ctrl_ops.open = wcn36xx_msm_smd_open; >> - wmsm.ctrl_ops.close = wcn36xx_msm_smd_close; >> - wmsm.ctrl_ops.tx = wcn36xx_msm_smd_send_and_wait; >> - wmsm.ctrl_ops.get_hw_mac = wcn36xx_msm_get_hw_mac; >> - wmsm.ctrl_ops.smsm_change_state = wcn36xx_msm_smsm_change_state; >> - wcnss_memory = >> - platform_get_resource_byname(pdev, >> - IORESOURCE_MEM, >> - "wcnss_mmio"); >> - if (wcnss_memory == NULL) { >> - dev_err(&wmsm.core->dev, >> - "Failed to get wcnss wlan memory map.\n"); >> - ret = -ENOMEM; >> - return ret; >> - } >> - memcpy(&res[0], wcnss_memory, sizeof(*wcnss_memory)); >> - >> - tx_irq = platform_get_resource_byname(pdev, >> - IORESOURCE_IRQ, >> - "wcnss_wlantx_irq"); >> - if (tx_irq == NULL) { >> - dev_err(&wmsm.core->dev, "Failed to get wcnss tx_irq"); >> - ret = -ENOMEM; >> - return ret; >> - } >> - memcpy(&res[1], tx_irq, sizeof(*tx_irq)); >> - >> - rx_irq = platform_get_resource_byname(pdev, >> - IORESOURCE_IRQ, >> - "wcnss_wlanrx_irq"); >> - if (rx_irq == NULL) { >> - dev_err(&wmsm.core->dev, "Failed to get wcnss rx_irq"); >> - ret = -ENOMEM; >> - return ret; >> + for (n = 0; n < ARRAY_SIZE(rnames); n++) { >> + r = platform_get_resource_byname(pdev, rtype[n], rnames[n]); >> + if (!r) { >> + dev_err(&wmsm.core->dev, >> + "Missing resource %s'\n", rnames[n]); >> + ret = -ENOMEM; >> + return ret; >> + } >> + res[n] = *r; >> } >> - memcpy(&res[2], rx_irq, sizeof(*rx_irq)); >> >> - platform_device_add_resources(wmsm.core, res, ARRAY_SIZE(res)); >> + platform_device_add_resources(wmsm.core, res, n); >> >> ret = platform_device_add_data(wmsm.core, &wmsm.ctrl_ops, >> sizeof(wmsm.ctrl_ops)); >> @@ -319,10 +326,6 @@ static int wcn36xx_msm_remove(struct platform_device *pdev) >> return 0; >> } >> >> -static const struct of_device_id wcn36xx_msm_match_table[] = { >> - { .compatible = "qcom,wcn36xx" }, >> - { } >> -}; >> MODULE_DEVICE_TABLE(of, wcn36xx_msm_match_table); >> >> static struct platform_driver wcn36xx_msm_driver = { >> > > > > -- > Best regards, > Eugene