mbox series

[RFC,00/19] wilc: added driver for wilc module

Message ID 1537957525-11467-1-git-send-email-ajay.kathat@microchip.com (mailing list archive)
Headers show
Series wilc: added driver for wilc module | expand

Message

Ajay Singh Sept. 26, 2018, 10:25 a.m. UTC
This patch set contains the driver files from 'driver/staging/wilc1000'.
Renamed the driver from 'wilc1000' to 'wilc' to have generic name, as
the same driver will be used by other wilc family members.

The patch series is based on 'wireless-driver-next'.

This driver will add support for SDIO/SPI interface for wilc family.
For now only wilc1000 module support has been intergrated in this driver.


Ajay Singh (19):
  wilc: add coreconfigurator.h
  wilc: add coreconfigurator.c
  wilc: add host_interface.h
  wilc: add host_interface.c
  wilc: add wilc_wlan_if.h
  wilc: add wilc_wlan_cfg.h
  wilc: add wilc_wlan_cfg.c
  wilc: add wilc_wlan.h
  wilc: add wilc_wlan.c
  wilc: add wilc_wfi_netdevice.h
  wilc: add wilc_wfi_cfgoperations.h
  wilc: add wilc_wfi_cfgoperations.c
  wilc: add linux_wlan.c
  wilc: add linux_mon.c
  wilc: add wilc_spi.c
  wilc: add wilc_sdio.c
  wilc: updated DT device binding for wilc device
  wilc: add Makefile and Kconfig files for  wilc compilation
  wilc: added wilc module compilation in wireless Makefile & Kconfig

 .../net/wireless/microchip,wilc1000,sdio.txt       |   32 +
 .../net/wireless/microchip,wilc1000,spi.txt        |   26 +
 drivers/net/wireless/Kconfig                       |    1 +
 drivers/net/wireless/Makefile                      |    1 +
 drivers/net/wireless/microchip/Kconfig             |   14 +
 drivers/net/wireless/microchip/Makefile            |    1 +
 drivers/net/wireless/microchip/wilc/Kconfig        |   42 +
 drivers/net/wireless/microchip/wilc/Makefile       |   15 +
 .../net/wireless/microchip/wilc/coreconfigurator.c |  287 ++
 .../net/wireless/microchip/wilc/coreconfigurator.h |   81 +
 .../net/wireless/microchip/wilc/host_interface.c   | 3927 ++++++++++++++++++++
 .../net/wireless/microchip/wilc/host_interface.h   |  362 ++
 drivers/net/wireless/microchip/wilc/linux_mon.c    |  273 ++
 drivers/net/wireless/microchip/wilc/linux_wlan.c   | 1161 ++++++
 drivers/net/wireless/microchip/wilc/wilc_sdio.c    | 1124 ++++++
 drivers/net/wireless/microchip/wilc/wilc_spi.c     | 1137 ++++++
 .../microchip/wilc/wilc_wfi_cfgoperations.c        | 2216 +++++++++++
 .../microchip/wilc/wilc_wfi_cfgoperations.h        |   23 +
 .../wireless/microchip/wilc/wilc_wfi_netdevice.h   |  230 ++
 drivers/net/wireless/microchip/wilc/wilc_wlan.c    | 1350 +++++++
 drivers/net/wireless/microchip/wilc/wilc_wlan.h    |  297 ++
 .../net/wireless/microchip/wilc/wilc_wlan_cfg.c    |  497 +++
 .../net/wireless/microchip/wilc/wilc_wlan_cfg.h    |   54 +
 drivers/net/wireless/microchip/wilc/wilc_wlan_if.h |  830 +++++
 drivers/staging/wilc1000/wilc_sdio.c               |    4 +-
 25 files changed, 13983 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/net/wireless/microchip,wilc1000,sdio.txt
 create mode 100644 Documentation/devicetree/bindings/net/wireless/microchip,wilc1000,spi.txt
 create mode 100644 drivers/net/wireless/microchip/Kconfig
 create mode 100644 drivers/net/wireless/microchip/Makefile
 create mode 100644 drivers/net/wireless/microchip/wilc/Kconfig
 create mode 100644 drivers/net/wireless/microchip/wilc/Makefile
 create mode 100644 drivers/net/wireless/microchip/wilc/coreconfigurator.c
 create mode 100644 drivers/net/wireless/microchip/wilc/coreconfigurator.h
 create mode 100644 drivers/net/wireless/microchip/wilc/host_interface.c
 create mode 100644 drivers/net/wireless/microchip/wilc/host_interface.h
 create mode 100644 drivers/net/wireless/microchip/wilc/linux_mon.c
 create mode 100644 drivers/net/wireless/microchip/wilc/linux_wlan.c
 create mode 100644 drivers/net/wireless/microchip/wilc/wilc_sdio.c
 create mode 100644 drivers/net/wireless/microchip/wilc/wilc_spi.c
 create mode 100644 drivers/net/wireless/microchip/wilc/wilc_wfi_cfgoperations.c
 create mode 100644 drivers/net/wireless/microchip/wilc/wilc_wfi_cfgoperations.h
 create mode 100644 drivers/net/wireless/microchip/wilc/wilc_wfi_netdevice.h
 create mode 100644 drivers/net/wireless/microchip/wilc/wilc_wlan.c
 create mode 100644 drivers/net/wireless/microchip/wilc/wilc_wlan.h
 create mode 100644 drivers/net/wireless/microchip/wilc/wilc_wlan_cfg.c
 create mode 100644 drivers/net/wireless/microchip/wilc/wilc_wlan_cfg.h
 create mode 100644 drivers/net/wireless/microchip/wilc/wilc_wlan_if.h

Comments

Kalle Valo Oct. 6, 2018, 12:45 p.m. UTC | #1
Ajay Singh <ajay.kathat@microchip.com> writes:

> This patch set contains the driver files from 'driver/staging/wilc1000'.
> Renamed the driver from 'wilc1000' to 'wilc' to have generic name, as
> the same driver will be used by other wilc family members.

I'm worried that the name 'wilc' is just too generic, I liked the
original name wilc1000 much more. Quite often when we have a new
generation of wireless devices there's also a new driver, so in the long
run I'm worried that a generic name like 'wilc' could be a source of
confusion. I think it's much smaller problem if 'wilc1000' (the driver)
also supports wilc3000 (the device), people are already used to that.
And if there's ever a need for a new driver, you can call it 'wilc4000'
or whatever.

For example, I think good driver names are like 'mt76' or 'rtw88'
(currently under review).
Ajay Singh Oct. 8, 2018, 5:17 a.m. UTC | #2
On Sat, 6 Oct 2018 15:45:41 +0300
Kalle Valo <kvalo@codeaurora.org> wrote:

> Ajay Singh <ajay.kathat@microchip.com> writes:
> 
> > This patch set contains the driver files from
> > 'driver/staging/wilc1000'. Renamed the driver from 'wilc1000' to
> > 'wilc' to have generic name, as the same driver will be used by
> > other wilc family members.  
> 
> I'm worried that the name 'wilc' is just too generic, I liked the
> original name wilc1000 much more. Quite often when we have a new
> generation of wireless devices there's also a new driver, so in the
> long run I'm worried that a generic name like 'wilc' could be a
> source of confusion. I think it's much smaller problem if
> 'wilc1000' (the driver) also supports wilc3000 (the device), people
> are already used to that. 

I also thought about retaining the same wilc1000 name, which has be
used for quite some time now. But as we are moving this driver
to '/driver/net/wireless/' freshly so thought it’s better to change in
the first commit.

As you know, 'wilc1000' name is used for folder name as well as module
name (i.e wilc1000.ko and wilc1000-spi.ko/wilc1000-sdio.ko). WILC3000 is
going to use the same modules(.ko).

Do you think its good approach to have 'wilc1000' folder but only remove
‘wilc1000’ prefix from .ko modules names in Makefile (i.e wilc.ko and
wilc-spio.ko/wilc-sdio.ko). So these .ko becomes generic and give clear
name to be used with both wilc1000 and wilc3000 modules.

Please let know what do you think about it and based on your input I
will make the change.

Also should I submit the v2 version by retaining old name or wait for
few more comments to include in that.

Regards,
Ajay
Kalle Valo Oct. 8, 2018, 7:38 a.m. UTC | #3
Ajay Singh <ajay.kathat@microchip.com> writes:

> On Sat, 6 Oct 2018 15:45:41 +0300
> Kalle Valo <kvalo@codeaurora.org> wrote:
>
>> Ajay Singh <ajay.kathat@microchip.com> writes:
>> 
>> > This patch set contains the driver files from
>> > 'driver/staging/wilc1000'. Renamed the driver from 'wilc1000' to
>> > 'wilc' to have generic name, as the same driver will be used by
>> > other wilc family members.  
>> 
>> I'm worried that the name 'wilc' is just too generic, I liked the
>> original name wilc1000 much more. Quite often when we have a new
>> generation of wireless devices there's also a new driver, so in the
>> long run I'm worried that a generic name like 'wilc' could be a
>> source of confusion. I think it's much smaller problem if
>> 'wilc1000' (the driver) also supports wilc3000 (the device), people
>> are already used to that. 
>
> I also thought about retaining the same wilc1000 name, which has be
> used for quite some time now. But as we are moving this driver
> to '/driver/net/wireless/' freshly so thought it’s better to change in
> the first commit.
>
> As you know, 'wilc1000' name is used for folder name as well as module
> name (i.e wilc1000.ko and wilc1000-spi.ko/wilc1000-sdio.ko). WILC3000 is
> going to use the same modules(.ko).

So just to be clear, this sounds good to me.

> Do you think its good approach to have 'wilc1000' folder but only remove
> ‘wilc1000’ prefix from .ko modules names in Makefile (i.e wilc.ko and
> wilc-spio.ko/wilc-sdio.ko). So these .ko becomes generic and give clear
> name to be used with both wilc1000 and wilc3000 modules.

But this I think is confusing. The driver should use the same name
everywhere, both in folder name ('wilc1000/') and in the module name
('wilc1000-*.ko').

If I'm understanding correctly you are worried that 'wilc1000-spi.ko'
also supports wilc3000 devices, but I don't see that as a problem. I
think it's very common (not just in wireless) that the marketing names
don't always match with driver names.

> Also should I submit the v2 version by retaining old name or wait for
> few more comments to include in that.

I haven't had a chance to review the driver and apparently nobody else
either. So I recommend waiting for review comments before submitting v2.

Also I suggest that you don't make any changes from the staging version,
like you did the rename. I think it's simplest to submit the code as it
is now in staging. And once you get review comments, first submit the
changes to staging and then send v2 for new review.
Adham Abozaeid Oct. 8, 2018, 6:34 p.m. UTC | #4
On 10/08/2018 12:38 AM, Kalle Valo wrote:
> Ajay Singh <ajay.kathat@microchip.com> writes:
> 
>> On Sat, 6 Oct 2018 15:45:41 +0300
>> Kalle Valo <kvalo@codeaurora.org> wrote:
>>
>>> Ajay Singh <ajay.kathat@microchip.com> writes:
>>>
>>>> This patch set contains the driver files from
>>>> 'driver/staging/wilc1000'. Renamed the driver from 'wilc1000' to
>>>> 'wilc' to have generic name, as the same driver will be used by
>>>> other wilc family members.  
>>>
>>> I'm worried that the name 'wilc' is just too generic, I liked the
>>> original name wilc1000 much more. Quite often when we have a new
>>> generation of wireless devices there's also a new driver, so in the
>>> long run I'm worried that a generic name like 'wilc' could be a
>>> source of confusion. I think it's much smaller problem if
>>> 'wilc1000' (the driver) also supports wilc3000 (the device), people
>>> are already used to that. 

> If I'm understanding correctly you are worried that 'wilc1000-spi.ko'
> also supports wilc3000 devices, but I don't see that as a problem. I
> think it's very common (not just in wireless) that the marketing names
> don't always match with driver names.
> 

It's highly unlikely that microchip will have a new generation of wilc devices other than wilc1000 and wilc3000, since a new family is already in development.
And in case a new generation was developed, it will be best to support it in the current driver because of the similarities between wilc devices.

I'm afraid that it might be more confusing for users to use wilc1000 naming while they are using wilc3000 hardware. It's not only that the name that is different from the marketing name, but it also refers to another existing product.

Thanks,
Adham
Kalle Valo Nov. 15, 2018, 2:11 p.m. UTC | #5
<Adham.Abozaeid@microchip.com> writes:

> On 10/08/2018 12:38 AM, Kalle Valo wrote:
>> Ajay Singh <ajay.kathat@microchip.com> writes:
>> 
>>> On Sat, 6 Oct 2018 15:45:41 +0300
>>> Kalle Valo <kvalo@codeaurora.org> wrote:
>>>
>>>> Ajay Singh <ajay.kathat@microchip.com> writes:
>>>>
>>>>> This patch set contains the driver files from
>>>>> 'driver/staging/wilc1000'. Renamed the driver from 'wilc1000' to
>>>>> 'wilc' to have generic name, as the same driver will be used by
>>>>> other wilc family members.  
>>>>
>>>> I'm worried that the name 'wilc' is just too generic, I liked the
>>>> original name wilc1000 much more. Quite often when we have a new
>>>> generation of wireless devices there's also a new driver, so in the
>>>> long run I'm worried that a generic name like 'wilc' could be a
>>>> source of confusion. I think it's much smaller problem if
>>>> 'wilc1000' (the driver) also supports wilc3000 (the device), people
>>>> are already used to that. 
>
>> If I'm understanding correctly you are worried that 'wilc1000-spi.ko'
>> also supports wilc3000 devices, but I don't see that as a problem. I
>> think it's very common (not just in wireless) that the marketing names
>> don't always match with driver names.
>> 
>
> It's highly unlikely that microchip will have a new generation of wilc
> devices other than wilc1000 and wilc3000, since a new family is
> already in development.
> And in case a new generation was developed, it will be best to support
> it in the current driver because of the similarities between wilc
> devices.
>
> I'm afraid that it might be more confusing for users to use wilc1000
> naming while they are using wilc3000 hardware. It's not only that the
> name that is different from the marketing name, but it also refers to
> another existing product.

Well, I see it very differently. For example, if I google 'wilc1000' I
get directly to the product page but with 'wilc' I get nothing useful.
And I have been dealing with marketing for so long that "never say
never" about what they will decide ;)

So I'm still not convinced that renaming is a good idea. But actually my
opinion doesn't matter here, as the rename should happen in the staging
tree (when the driver is moved from staging to drivers/net/wireless it
should be a simple rename, no other changes). So I'll leave this for
Greg to decide if the rename is worthwhile or not. My vote would be a
clear "no" :)