Re: [PATCH v3 00/19] rtlwifi: halmac: Add new module halmac

[Pkshih]

On Thu, 2018-05-24 at 11:27 +0300, Kalle Valo wrote:
> 
> You are missing my point: I don't even have time to review huge rtlwifi
> patches when they are not even ready for upstream. I cannot start
> working on cleaning up rtlwifi code and doing multiple iterations of
> reviews on these kind of huge patchsets. Either you need to
> significantly scale down the size of patchsets (especially LOC) or you
> need to get review help from someone else. But the current way of
> working is not doable for me.
> 

Is there a proper way to look for "someone else" you mentioned?

We plan to rewrite a new driver excluding agnostic OS layer to support 
new generation 11AC chips, because they're very different from the chips
existed in rtlwifi and rtl8xxxu. 

If we have a "someone" to review our driver, where is the proper place to
put developing driver repository? Staging or public git repository 
(e.g. GitHub)?

Finally, the driver is done. Are there explicit criteria to accept the
driver as a mainline driver? 

Thanks
PK

Re: [PATCH v3 00/19] rtlwifi: halmac: Add new module halmac

[Kalle Valo]

Pkshih <pks...@realtek.com> writes:

> On Wed, 2018-05-16 at 15:36 +0300, Kalle Valo wrote:
>> Pkshih <pks...@realtek.com> writes:
>> 
>> > On Mon, 2018-04-30 at 14:03 +0530, Krishna Chaitanya wrote:
>> >> On Mon, Apr 30, 2018 at 8:10 AM, Pkshih <pks...@realtek.com> wrote:
>> >> >
>> >> >
>> >> > > -Original Message-
>> >> > > From: Barry Day [mailto:brise...@gmail.com]
>> >> > > Sent: Saturday, April 28, 2018 6:42 AM
>> >> > > To: Pkshih
>> >> > > Cc: Kalle Valo; larry.fin...@lwfinger.net; 
>> >> > > linux-wireless@vger.kernel.org
>> >> > > Subject: Re: [PATCH v3 00/19] rtlwifi: halmac: Add new module halmac
>> >> > >
>> >> > > On Fri, Apr 27, 2018 at 05:44:16AM +, Pkshih wrote:
>> >> > > >
>> >> > > > The registers reside in driver causes error frequently, because MAC 
>> >> > > > register
>> >> > > > is maintained by Realtek's MAC team so they create this module to 
>> >> > > > avoid mistakes.
>> >> > > > Another benefit is to make it possible to become a thin driver, 
>> >> > > > because many
>> >> > > > common functions are provided, so duplicate code will be reduced.
>> >> > >
>> >> > > How is it possible to create a thin driver by adding lots more code 
>> >> > > and layers
>> >> > > of indirection ??? and writing it in a way that it won't compile 
>> >> > > without the
>> >> > > code for every type of bus interface even though most modules only 
>> >> > > use one ?
>> >> > >
>> >> > As I mentioned in first paragraph "(I use 'driver' in this mail 
>> >> > indicates part of
>> >> > rtlwifi excluded from this module.)". If this module was seen as a 
>> >> > 'lib', rtl8822be
>> >> > would be a "thin driver". For bus interface code, I need to add a way 
>> >> > to compile
>> >> > type of bus interface according to selected chip.
>> >> >
>> >> > > It's a horrible pile of garbage slapped together by an inexperienced
>> >> > > programmer. Its a major deterrent for anyone looking at working on 
>> >> > > one of
>> >> > > the latest realtek drivers.
>> >> > >
>> >> > This module is designed to support multiple OS including Windows and 
>> >> > Linux, and
>> >> > many products have used this module and worked well. We hope Linux user 
>> >> > can also
>> >> > use Realtek's WiFi without additional installation if driver was built.
>> >> > In order to submit this module to kernel upstream, we take a lot of 
>> >> > effort
>> >> > to fit Linux coding conventions (e.g. coding style), and explicit
>> >> > suggestions will be helpful for us to continuously improve this module.
>> >> 
>> >> IMHO, this is a common use case for most organizations. I understand
>> >> that Linux cannot
>> >> accommodate other OSes requirements but is there an approved/recommended 
>> >> way
>> >> to upstream an OS agnostic driver? Agnostic drivers are generally
>> >> bulkier compared to
>> >> Linux-only drivers and also code organization is also different (to
>> >> handle other OSes).
>> >> 
>> >
>> > Hi Kalle,
>> >
>> > The state of this patchset was changed to RFC in patchwork, and I look at 
>> > RFC's
>> > meaning in wireless wiki. Do you expect that I will send v4?
>> 
>> Yes, I was expecting that you will submit v4 with proper documentation.
>> I was supposed to send an email but forgot, sorry.
>> 
>> > If so, what do I need to fix in v4? Or, you need more description
>> > about this module, please let me know. 
>> 
>> The biggest problem is that rtlwifi patches are way too big and which I
>> don't think are ready for upstream, most of the time code quality is
>> closer to the infamous "vendor drivers". This is causing me too much
>> burden, even just reviewing and providing initial comments to rtlwifi
>> patches take too much of my time. For example, I still haven't been able
>> to check the rtlwifi btcoex patches from a month ago.
>> 
&g

Re: [PATCH v3 00/19] rtlwifi: halmac: Add new module halmac

[Pkshih]

On Wed, 2018-05-16 at 15:36 +0300, Kalle Valo wrote:
> Pkshih <pks...@realtek.com> writes:
> 
> > On Mon, 2018-04-30 at 14:03 +0530, Krishna Chaitanya wrote:
> >> On Mon, Apr 30, 2018 at 8:10 AM, Pkshih <pks...@realtek.com> wrote:
> >> >
> >> >
> >> > > -Original Message-
> >> > > From: Barry Day [mailto:brise...@gmail.com]
> >> > > Sent: Saturday, April 28, 2018 6:42 AM
> >> > > To: Pkshih
> >> > > Cc: Kalle Valo; larry.fin...@lwfinger.net; 
> >> > > linux-wireless@vger.kernel.org
> >> > > Subject: Re: [PATCH v3 00/19] rtlwifi: halmac: Add new module halmac
> >> > >
> >> > > On Fri, Apr 27, 2018 at 05:44:16AM +, Pkshih wrote:
> >> > > >
> >> > > > The registers reside in driver causes error frequently, because MAC 
> >> > > > register
> >> > > > is maintained by Realtek's MAC team so they create this module to 
> >> > > > avoid mistakes.
> >> > > > Another benefit is to make it possible to become a thin driver, 
> >> > > > because many
> >> > > > common functions are provided, so duplicate code will be reduced.
> >> > >
> >> > > How is it possible to create a thin driver by adding lots more code 
> >> > > and layers
> >> > > of indirection ??? and writing it in a way that it won't compile 
> >> > > without the
> >> > > code for every type of bus interface even though most modules only use 
> >> > > one ?
> >> > >
> >> > As I mentioned in first paragraph "(I use 'driver' in this mail 
> >> > indicates part of
> >> > rtlwifi excluded from this module.)". If this module was seen as a 
> >> > 'lib', rtl8822be
> >> > would be a "thin driver". For bus interface code, I need to add a way to 
> >> > compile
> >> > type of bus interface according to selected chip.
> >> >
> >> > > It's a horrible pile of garbage slapped together by an inexperienced
> >> > > programmer. Its a major deterrent for anyone looking at working on one 
> >> > > of
> >> > > the latest realtek drivers.
> >> > >
> >> > This module is designed to support multiple OS including Windows and 
> >> > Linux, and
> >> > many products have used this module and worked well. We hope Linux user 
> >> > can also
> >> > use Realtek's WiFi without additional installation if driver was built.
> >> > In order to submit this module to kernel upstream, we take a lot of 
> >> > effort
> >> > to fit Linux coding conventions (e.g. coding style), and explicit
> >> > suggestions will be helpful for us to continuously improve this module.
> >> 
> >> IMHO, this is a common use case for most organizations. I understand
> >> that Linux cannot
> >> accommodate other OSes requirements but is there an approved/recommended 
> >> way
> >> to upstream an OS agnostic driver? Agnostic drivers are generally
> >> bulkier compared to
> >> Linux-only drivers and also code organization is also different (to
> >> handle other OSes).
> >> 
> >
> > Hi Kalle,
> >
> > The state of this patchset was changed to RFC in patchwork, and I look at 
> > RFC's
> > meaning in wireless wiki. Do you expect that I will send v4?
> 
> Yes, I was expecting that you will submit v4 with proper documentation.
> I was supposed to send an email but forgot, sorry.
> 
> > If so, what do I need to fix in v4? Or, you need more description
> > about this module, please let me know. 
> 
> The biggest problem is that rtlwifi patches are way too big and which I
> don't think are ready for upstream, most of the time code quality is
> closer to the infamous "vendor drivers". This is causing me too much
> burden, even just reviewing and providing initial comments to rtlwifi
> patches take too much of my time. For example, I still haven't been able
> to check the rtlwifi btcoex patches from a month ago.
> 
> In principle I can use a minute or two per patch, anything longer than
> that and I can't keep up with the incoming patch flow. And with huge
> rtlwifi patchsets I usually need something more like an hour than few
> minutes.
> 
> I have said this also before, but more and more I'm thinking that
> rtlwifi is not really a proper upstream driver. I think staging would be
> a much better place for it and maybe a proper upstream realtek driver
> would be something based on rtl8xxxu? I dunno.
> 
> But we really need to find a solution for this as the current way with
> rtlwifi patches won't work in the long run.
> 

If we remove unused code and do proper modification (e.g. remove abstraction 
layer)
and submit to staging, but still remain the directory levels.
Will you accept halmac and submit it into upstream after being reviewed in 
staging?
Or, the only way you can accept is to remove the halmac directory and rearrange
the code and split it into the top level directory?

Regards,
PK

Re: [PATCH v3 00/19] rtlwifi: halmac: Add new module halmac

[Kalle Valo]

Pkshih  writes:

>> And I even read all 19 commit logs and there was no mention of why this
>> is needed either. I cannot just blindly apply patches without knowing
>> what they do.
>> 
>
> This new module halmac is an abstract layer for Realtek WiFi MAC to provide
> common interfaces to access WiFi MAC and register set. If driver (I use 
> 'driver'
> in this mail indicates part of rtlwifi excluded from this module.) uses the 
> API,
> it's not necessary to control related register directly. If API isn't 
> implemented,
> driver can use the registers defined by halmac to manipulate MAC function.
>
> The registers reside in driver causes error frequently, because MAC register
> is maintained by Realtek's MAC team so they create this module to avoid 
> mistakes.
> Another benefit is to make it possible to become a thin driver, because many
> common functions are provided, so duplicate code will be reduced.
>
> Currently, this module is aim to rtl8822be, and it will extend to support
> more chips so hierarchical directory is adopted. It looks like
>rtlwifi/halmac/halmac_88xx
>rtlwifi/halmac/halmac_88xx/halmac_8822b
>rtlwifi/halmac/halmac_88xx/halmac_8821c (future chip)

IMHO the directory structure contains redundant information, it's enough
to have halmac only once in the path. So I would instead use this kind
of naming:

rtlwifi/halmac/88xx/8822b

-- 
Kalle Valo

Re: [PATCH v3 00/19] rtlwifi: halmac: Add new module halmac

[Kalle Valo]

Krishna Chaitanya <chaitanya.m...@gmail.com> writes:

> On Mon, Apr 30, 2018 at 8:10 AM, Pkshih <pks...@realtek.com> wrote:
>>
>>
>> > -Original Message-
>> > From: Barry Day [mailto:brise...@gmail.com]
>> > Sent: Saturday, April 28, 2018 6:42 AM
>> > To: Pkshih
>> > Cc: Kalle Valo; larry.fin...@lwfinger.net; linux-wireless@vger.kernel.org
>> > Subject: Re: [PATCH v3 00/19] rtlwifi: halmac: Add new module halmac
>> >
>> > On Fri, Apr 27, 2018 at 05:44:16AM +, Pkshih wrote:
>> > >
>> > > The registers reside in driver causes error frequently, because MAC 
>> > > register
>> > > is maintained by Realtek's MAC team so they create this module to avoid 
>> > > mistakes.
>> > > Another benefit is to make it possible to become a thin driver, because 
>> > > many
>> > > common functions are provided, so duplicate code will be reduced.
>> >
>> > How is it possible to create a thin driver by adding lots more code and 
>> > layers
>> > of indirection ??? and writing it in a way that it won't compile without 
>> > the
>> > code for every type of bus interface even though most modules only use one 
>> > ?
>> >
>> As I mentioned in first paragraph "(I use 'driver' in this mail indicates 
>> part of
>> rtlwifi excluded from this module.)". If this module was seen as a 'lib', 
>> rtl8822be
>> would be a "thin driver". For bus interface code, I need to add a way to 
>> compile
>> type of bus interface according to selected chip.
>>
>> > It's a horrible pile of garbage slapped together by an inexperienced
>> > programmer. Its a major deterrent for anyone looking at working on one of
>> > the latest realtek drivers.
>> >
>> This module is designed to support multiple OS including Windows and Linux, 
>> and
>> many products have used this module and worked well. We hope Linux user can 
>> also
>> use Realtek's WiFi without additional installation if driver was built.
>> In order to submit this module to kernel upstream, we take a lot of effort
>> to fit Linux coding conventions (e.g. coding style), and explicit
>> suggestions will be helpful for us to continuously improve this module.
>
> IMHO, this is a common use case for most organizations.

Yes, it is.

> I understand that Linux cannot accommodate other OSes requirements but
> is there an approved/recommended way to upstream an OS agnostic
> driver?

When having an OS agnostic driver My recommendataion is to rewrite the
driver :) We have plenty of examples doing that: wl1251/wl18xx, ath10k,
rtl8xxx, cw1200 and wcn36xx at least. But of course it's possible to
major cleanup to the driver and then get it accepted, with ath6kl we did
that and I think brcmfmac also did that.

> Agnostic drivers are generally bulkier compared to Linux-only drivers
> and also code organization is also different (to handle other OSes).

Indeed, I hate those OS agnostic vendor drivers so much. So many
different layers and abstractions make my head just spin. Luckily we
seem to have proper Linux upstream drivers for most of the wireless
hardware nowadays!

-- 
Kalle Valo

Re: [PATCH v3 00/19] rtlwifi: halmac: Add new module halmac

[Pkshih]

On Mon, 2018-04-30 at 14:03 +0530, Krishna Chaitanya wrote:
> On Mon, Apr 30, 2018 at 8:10 AM, Pkshih <pks...@realtek.com> wrote:
> >
> >
> > > -Original Message-
> > > From: Barry Day [mailto:brise...@gmail.com]
> > > Sent: Saturday, April 28, 2018 6:42 AM
> > > To: Pkshih
> > > Cc: Kalle Valo; larry.fin...@lwfinger.net; linux-wireless@vger.kernel.org
> > > Subject: Re: [PATCH v3 00/19] rtlwifi: halmac: Add new module halmac
> > >
> > > On Fri, Apr 27, 2018 at 05:44:16AM +, Pkshih wrote:
> > > >
> > > > The registers reside in driver causes error frequently, because MAC 
> > > > register
> > > > is maintained by Realtek's MAC team so they create this module to avoid 
> > > > mistakes.
> > > > Another benefit is to make it possible to become a thin driver, because 
> > > > many
> > > > common functions are provided, so duplicate code will be reduced.
> > >
> > > How is it possible to create a thin driver by adding lots more code and 
> > > layers
> > > of indirection ??? and writing it in a way that it won't compile without 
> > > the
> > > code for every type of bus interface even though most modules only use 
> > > one ?
> > >
> > As I mentioned in first paragraph "(I use 'driver' in this mail indicates 
> > part of
> > rtlwifi excluded from this module.)". If this module was seen as a 'lib', 
> > rtl8822be
> > would be a "thin driver". For bus interface code, I need to add a way to 
> > compile
> > type of bus interface according to selected chip.
> >
> > > It's a horrible pile of garbage slapped together by an inexperienced
> > > programmer. Its a major deterrent for anyone looking at working on one of
> > > the latest realtek drivers.
> > >
> > This module is designed to support multiple OS including Windows and Linux, 
> > and
> > many products have used this module and worked well. We hope Linux user can 
> > also
> > use Realtek's WiFi without additional installation if driver was built.
> > In order to submit this module to kernel upstream, we take a lot of effort
> > to fit Linux coding conventions (e.g. coding style), and explicit
> > suggestions will be helpful for us to continuously improve this module.
> 
> IMHO, this is a common use case for most organizations. I understand
> that Linux cannot
> accommodate other OSes requirements but is there an approved/recommended way
> to upstream an OS agnostic driver? Agnostic drivers are generally
> bulkier compared to
> Linux-only drivers and also code organization is also different (to
> handle other OSes).
> 

Hi Kalle,

The state of this patchset was changed to RFC in patchwork, and I look at RFC's
meaning in wireless wiki. Do you expect that I will send v4? If so, what do I
need to fix in v4? Or, you need more description about this module, please let
me know. 

Thanks
PK

Re: [PATCH v3 00/19] rtlwifi: halmac: Add new module halmac

[Krishna Chaitanya]

On Mon, Apr 30, 2018 at 8:10 AM, Pkshih <pks...@realtek.com> wrote:
>
>
> > -Original Message-
> > From: Barry Day [mailto:brise...@gmail.com]
> > Sent: Saturday, April 28, 2018 6:42 AM
> > To: Pkshih
> > Cc: Kalle Valo; larry.fin...@lwfinger.net; linux-wireless@vger.kernel.org
> > Subject: Re: [PATCH v3 00/19] rtlwifi: halmac: Add new module halmac
> >
> > On Fri, Apr 27, 2018 at 05:44:16AM +, Pkshih wrote:
> > >
> > > The registers reside in driver causes error frequently, because MAC 
> > > register
> > > is maintained by Realtek's MAC team so they create this module to avoid 
> > > mistakes.
> > > Another benefit is to make it possible to become a thin driver, because 
> > > many
> > > common functions are provided, so duplicate code will be reduced.
> >
> > How is it possible to create a thin driver by adding lots more code and 
> > layers
> > of indirection ??? and writing it in a way that it won't compile without the
> > code for every type of bus interface even though most modules only use one ?
> >
> As I mentioned in first paragraph "(I use 'driver' in this mail indicates 
> part of
> rtlwifi excluded from this module.)". If this module was seen as a 'lib', 
> rtl8822be
> would be a "thin driver". For bus interface code, I need to add a way to 
> compile
> type of bus interface according to selected chip.
>
> > It's a horrible pile of garbage slapped together by an inexperienced
> > programmer. Its a major deterrent for anyone looking at working on one of
> > the latest realtek drivers.
> >
> This module is designed to support multiple OS including Windows and Linux, 
> and
> many products have used this module and worked well. We hope Linux user can 
> also
> use Realtek's WiFi without additional installation if driver was built.
> In order to submit this module to kernel upstream, we take a lot of effort
> to fit Linux coding conventions (e.g. coding style), and explicit
> suggestions will be helpful for us to continuously improve this module.

IMHO, this is a common use case for most organizations. I understand
that Linux cannot
accommodate other OSes requirements but is there an approved/recommended way
to upstream an OS agnostic driver? Agnostic drivers are generally
bulkier compared to
Linux-only drivers and also code organization is also different (to
handle other OSes).

RE: [PATCH v3 00/19] rtlwifi: halmac: Add new module halmac

[Pkshih]

> -Original Message-
> From: Barry Day [mailto:brise...@gmail.com]
> Sent: Saturday, April 28, 2018 6:42 AM
> To: Pkshih
> Cc: Kalle Valo; larry.fin...@lwfinger.net; linux-wireless@vger.kernel.org
> Subject: Re: [PATCH v3 00/19] rtlwifi: halmac: Add new module halmac
> 
> On Fri, Apr 27, 2018 at 05:44:16AM +, Pkshih wrote:
> >
> > The registers reside in driver causes error frequently, because MAC register
> > is maintained by Realtek's MAC team so they create this module to avoid 
> > mistakes.
> > Another benefit is to make it possible to become a thin driver, because many
> > common functions are provided, so duplicate code will be reduced.
> 
> How is it possible to create a thin driver by adding lots more code and layers
> of indirection ??? and writing it in a way that it won't compile without the
> code for every type of bus interface even though most modules only use one ?
> 
As I mentioned in first paragraph "(I use 'driver' in this mail indicates part 
of
rtlwifi excluded from this module.)". If this module was seen as a 'lib', 
rtl8822be
would be a "thin driver". For bus interface code, I need to add a way to 
compile 
type of bus interface according to selected chip.

> It's a horrible pile of garbage slapped together by an inexperienced
> programmer. Its a major deterrent for anyone looking at working on one of
> the latest realtek drivers.
> 
This module is designed to support multiple OS including Windows and Linux, and 
many products have used this module and worked well. We hope Linux user can also
use Realtek's WiFi without additional installation if driver was built.
In order to submit this module to kernel upstream, we take a lot of effort 
to fit Linux coding conventions (e.g. coding style), and explicit 
suggestions will be helpful for us to continuously improve this module.

Thanks
PK

Re: [PATCH v3 00/19] rtlwifi: halmac: Add new module halmac

[Barry Day]

On Fri, Apr 27, 2018 at 05:44:16AM +, Pkshih wrote:
>
> The registers reside in driver causes error frequently, because MAC register
> is maintained by Realtek's MAC team so they create this module to avoid 
> mistakes.
> Another benefit is to make it possible to become a thin driver, because many
> common functions are provided, so duplicate code will be reduced.

How is it possible to create a thin driver by adding lots more code and layers
of indirection ??? and writing it in a way that it won't compile without the
code for every type of bus interface even though most modules only use one ?

It's a horrible pile of garbage slapped together by an inexperienced
programmer. Its a major deterrent for anyone looking at working on one of
the latest realtek drivers.

RE: [PATCH v3 00/19] rtlwifi: halmac: Add new module halmac

[Pkshih]

> -Original Message-
> From: Kalle Valo [mailto:kv...@codeaurora.org]
> Sent: Wednesday, April 25, 2018 3:36 PM
> To: Pkshih
> Cc: larry.fin...@lwfinger.net; linux-wireless@vger.kernel.org
> Subject: Re: [PATCH v3 00/19] rtlwifi: halmac: Add new module halmac
> 
> <pks...@realtek.com> writes:
> 
> > From: Ping-Ke Shih <pks...@realtek.com>
> >
> > v3: patch 06/17 in v2 is too big, so mail server may block the mail.
> > In this patchset, I decompose the patch into three patches 06/19-08/19,
> > so in patchset view v2 and v3 are identical.
> 
> I can confirm, all patches made it to patchwork now.
> 
> > v2: remove indirection to get halmac ops
> > Only patches 1/17 and 11/17 are changed.
> >
> > Patches 1/19-3/19 are added structure to support this module.
> > Patches 4/19-18/19 add new files.
> > Patch 19/19 add this module to Makefile and Kconfig.
> >
> > Ping-Ke Shih (19):
> >   rtlwifi: add halmac structure to wifi.h
> >   rtlwifi: add debug ID COMP_HALMAC
> >   rtlwifi: add dmdef.h to share with driver and other modules
> >   rtlwifi: halmac: add main definition used by halmac
> >   rtlwifi: halmac: describe number and size of chip functions
> >   rtlwifi: halmac: add register definitions
> >   rtlwifi: halmac: add bit field definitions
> >   rtlwifi: halmac: add bit field definitions of rtl8822b
> >   rtlwifi: halmac: add definition of TX/RX descriptor
> >   rtlwifi: halmac: add GPIO pin/pinmux definitions
> >   rtlwifi: halmac: add power sequence to turn on/off wifi card
> >   rtlwifi: halmac: access efuse through halmac helper functions
> >   rtlwifi: halmac: add files to implement halmac ops
> >   rtlwifi: halmac: add halmac init/deinit functions
> >   rtlwifi: halmac: add firmware related functions and definitions
> >   rtlwifi: halmac: add bus interface commands
> >   rtlwifi: halmac: add to control WiFi mac functions and registers
> >   rtlwifi: halmac: add to support BB and RF functions
> >   rtlwifi: add halmac to Makefile and Kconfig
> 
> You are adding a new component to rtlwifi but there's no introduction in
> the cover letter. Why is this needed? For what hardware is it for? What
> new features does it bring?
> 
> >  77 files changed, 64201 insertions(+)
> 
> This is a huge patchset, I'm not even sure if I want to start reviewing
> this. Especially when I have no clue about the big picture (which should
> be described in the cover letter).
> 
> https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches#too_many_patches
> 
I know the number of patches in a patchset should be within 10-12, but I
think that all files are new and full source can give you whole picture
if you want to know code flow between them.

> And I even read all 19 commit logs and there was no mention of why this
> is needed either. I cannot just blindly apply patches without knowing
> what they do.
> 

This new module halmac is an abstract layer for Realtek WiFi MAC to provide
common interfaces to access WiFi MAC and register set. If driver (I use 'driver'
in this mail indicates part of rtlwifi excluded from this module.) uses the API,
it's not necessary to control related register directly. If API isn't 
implemented,
driver can use the registers defined by halmac to manipulate MAC function.

The registers reside in driver causes error frequently, because MAC register
is maintained by Realtek's MAC team so they create this module to avoid 
mistakes.
Another benefit is to make it possible to become a thin driver, because many
common functions are provided, so duplicate code will be reduced.

Currently, this module is aim to rtl8822be, and it will extend to support
more chips so hierarchical directory is adopted. It looks like
   rtlwifi/halmac/halmac_88xx
   rtlwifi/halmac/halmac_88xx/halmac_8822b
   rtlwifi/halmac/halmac_88xx/halmac_8821c (future chip)
The directory halmac_88xx puts common functions of rtl8822b and rtl8821c, and
halmac_88xx/halmac_8822b puts rtl8822b specific functions.
To access API abstractly, it provides hook pointers to specific chip during
initialization.


In order to illustrate the functions, I list *.c files and short description:

   halmac/rtl_halmac.c
 We implement a facade-like (facade is a term in design pattern) by
 halmac_ops in this file, so we can check halmac_ops to know halmac 
functions.
 The functions, for examples, are init/deinit, power on/off, read efuse,
 c2h handler, iqk assistant function etc. Each function may call one or 
more 
 halmac API to achieve.
  
   halmac/halmac_api.c
   halmac/halmac_88xx/halmac_init_88xx.c
   halmac/halmac_88xx/halmac_8822b/halmac_init_8822b.c
 The init/deinit and mount ho

Re: [PATCH v3 00/19] rtlwifi: halmac: Add new module halmac

[Kalle Valo]

 writes:

> From: Ping-Ke Shih 
>
> v3: patch 06/17 in v2 is too big, so mail server may block the mail.
> In this patchset, I decompose the patch into three patches 06/19-08/19,
> so in patchset view v2 and v3 are identical.

I can confirm, all patches made it to patchwork now.

> v2: remove indirection to get halmac ops
> Only patches 1/17 and 11/17 are changed.
>
> Patches 1/19-3/19 are added structure to support this module.
> Patches 4/19-18/19 add new files.
> Patch 19/19 add this module to Makefile and Kconfig.
>
> Ping-Ke Shih (19):
>   rtlwifi: add halmac structure to wifi.h
>   rtlwifi: add debug ID COMP_HALMAC
>   rtlwifi: add dmdef.h to share with driver and other modules
>   rtlwifi: halmac: add main definition used by halmac
>   rtlwifi: halmac: describe number and size of chip functions
>   rtlwifi: halmac: add register definitions
>   rtlwifi: halmac: add bit field definitions
>   rtlwifi: halmac: add bit field definitions of rtl8822b
>   rtlwifi: halmac: add definition of TX/RX descriptor
>   rtlwifi: halmac: add GPIO pin/pinmux definitions
>   rtlwifi: halmac: add power sequence to turn on/off wifi card
>   rtlwifi: halmac: access efuse through halmac helper functions
>   rtlwifi: halmac: add files to implement halmac ops
>   rtlwifi: halmac: add halmac init/deinit functions
>   rtlwifi: halmac: add firmware related functions and definitions
>   rtlwifi: halmac: add bus interface commands
>   rtlwifi: halmac: add to control WiFi mac functions and registers
>   rtlwifi: halmac: add to support BB and RF functions
>   rtlwifi: add halmac to Makefile and Kconfig

You are adding a new component to rtlwifi but there's no introduction in
the cover letter. Why is this needed? For what hardware is it for? What
new features does it bring?

>  77 files changed, 64201 insertions(+)

This is a huge patchset, I'm not even sure if I want to start reviewing
this. Especially when I have no clue about the big picture (which should
be described in the cover letter). 

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches#too_many_patches

And I even read all 19 commit logs and there was no mention of why this
is needed either. I cannot just blindly apply patches without knowing
what they do.

-- 
Kalle Valo

[PATCH v3 00/19] rtlwifi: halmac: Add new module halmac

[pkshih]

From: Ping-Ke Shih 

v3: patch 06/17 in v2 is too big, so mail server may block the mail.
In this patchset, I decompose the patch into three patches 06/19-08/19,
so in patchset view v2 and v3 are identical.

v2: remove indirection to get halmac ops
Only patches 1/17 and 11/17 are changed.

Patches 1/19-3/19 are added structure to support this module.
Patches 4/19-18/19 add new files.
Patch 19/19 add this module to Makefile and Kconfig.

Ping-Ke Shih (19):
  rtlwifi: add halmac structure to wifi.h
  rtlwifi: add debug ID COMP_HALMAC
  rtlwifi: add dmdef.h to share with driver and other modules
  rtlwifi: halmac: add main definition used by halmac
  rtlwifi: halmac: describe number and size of chip functions
  rtlwifi: halmac: add register definitions
  rtlwifi: halmac: add bit field definitions
  rtlwifi: halmac: add bit field definitions of rtl8822b
  rtlwifi: halmac: add definition of TX/RX descriptor
  rtlwifi: halmac: add GPIO pin/pinmux definitions
  rtlwifi: halmac: add power sequence to turn on/off wifi card
  rtlwifi: halmac: access efuse through halmac helper functions
  rtlwifi: halmac: add files to implement halmac ops
  rtlwifi: halmac: add halmac init/deinit functions
  rtlwifi: halmac: add firmware related functions and definitions
  rtlwifi: halmac: add bus interface commands
  rtlwifi: halmac: add to control WiFi mac functions and registers
  rtlwifi: halmac: add to support BB and RF functions
  rtlwifi: add halmac to Makefile and Kconfig

 drivers/net/wireless/realtek/rtlwifi/Kconfig   | 5 +
 drivers/net/wireless/realtek/rtlwifi/Makefile  | 1 +
 drivers/net/wireless/realtek/rtlwifi/debug.h   | 1 +
 drivers/net/wireless/realtek/rtlwifi/dmdef.h   |40 +
 .../net/wireless/realtek/rtlwifi/halmac/Makefile   |29 +
 .../halmac_88xx/halmac_8822b/halmac_8822b_cfg.h|68 +
 .../halmac_8822b/halmac_cfg_wmac_8822b.c   |   144 +
 .../halmac_8822b/halmac_cfg_wmac_8822b.h   |36 +
 .../halmac_88xx/halmac_8822b/halmac_common_8822b.c |   173 +
 .../halmac_88xx/halmac_8822b/halmac_common_8822b.h |32 +
 .../halmac_88xx/halmac_8822b/halmac_gpio_8822b.c   |   847 +
 .../halmac_88xx/halmac_8822b/halmac_gpio_8822b.h   |34 +
 .../halmac_88xx/halmac_8822b/halmac_init_8822b.c   |   688 +
 .../halmac_88xx/halmac_8822b/halmac_init_8822b.h   |33 +
 .../halmac_88xx/halmac_8822b/halmac_pcie_8822b.c   |   218 +
 .../halmac_88xx/halmac_8822b/halmac_pcie_8822b.h   |38 +
 .../halmac_88xx/halmac_8822b/halmac_phy_8822b.c|   148 +
 .../halmac_8822b/halmac_pwr_seq_8822b.c|   396 +
 .../halmac_8822b/halmac_pwr_seq_8822b.h|26 +
 .../halmac_88xx/halmac_8822b/halmac_sdio_8822b.c   |   880 +
 .../halmac_88xx/halmac_8822b/halmac_sdio_8822b.h   |62 +
 .../halmac_88xx/halmac_8822b/halmac_usb_8822b.c|   161 +
 .../halmac_88xx/halmac_8822b/halmac_usb_8822b.h|38 +
 .../rtlwifi/halmac/halmac_88xx/halmac_88xx_cfg.h   |40 +
 .../rtlwifi/halmac/halmac_88xx/halmac_bb_rf_88xx.c |   397 +
 .../rtlwifi/halmac/halmac_88xx/halmac_bb_rf_88xx.h |53 +
 .../halmac/halmac_88xx/halmac_cfg_wmac_88xx.c  |  1158 ++
 .../halmac/halmac_88xx/halmac_cfg_wmac_88xx.h  |   122 +
 .../halmac/halmac_88xx/halmac_common_88xx.c|  2931 +++
 .../halmac/halmac_88xx/halmac_common_88xx.h|   151 +
 .../rtlwifi/halmac/halmac_88xx/halmac_efuse_88xx.c |  1918 ++
 .../rtlwifi/halmac/halmac_88xx/halmac_efuse_88xx.h |   101 +
 .../rtlwifi/halmac/halmac_88xx/halmac_flash_88xx.c |   316 +
 .../rtlwifi/halmac/halmac_88xx/halmac_flash_88xx.h |35 +
 .../rtlwifi/halmac/halmac_88xx/halmac_fw_88xx.c|  1149 ++
 .../rtlwifi/halmac/halmac_88xx/halmac_fw_88xx.h|57 +
 .../rtlwifi/halmac/halmac_88xx/halmac_gpio_88xx.c  |   415 +
 .../rtlwifi/halmac/halmac_88xx/halmac_gpio_88xx.h  |55 +
 .../rtlwifi/halmac/halmac_88xx/halmac_init_88xx.c  |  1071 ++
 .../rtlwifi/halmac/halmac_88xx/halmac_init_88xx.h  |70 +
 .../rtlwifi/halmac/halmac_88xx/halmac_mimo_88xx.c  |   869 +
 .../rtlwifi/halmac/halmac_88xx/halmac_mimo_88xx.h  |79 +
 .../rtlwifi/halmac/halmac_88xx/halmac_pcie_88xx.c  |   538 +
 .../rtlwifi/halmac/halmac_88xx/halmac_pcie_88xx.h  |98 +
 .../rtlwifi/halmac/halmac_88xx/halmac_sdio_88xx.c  |   895 +
 .../rtlwifi/halmac/halmac_88xx/halmac_sdio_88xx.h  |75 +
 .../rtlwifi/halmac/halmac_88xx/halmac_usb_88xx.c   |   522 +
 .../rtlwifi/halmac/halmac_88xx/halmac_usb_88xx.h   |83 +
 .../wireless/realtek/rtlwifi/halmac/halmac_api.c   |   438 +
 .../wireless/realtek/rtlwifi/halmac/halmac_api.h   |65 +
 .../wireless/realtek/rtlwifi/halmac/halmac_bit2.h  | 18234 +++
 .../realtek/rtlwifi/halmac/halmac_bit_8822b.h  | 17870 ++
 .../realtek/rtlwifi/halmac/halmac_fw_info.h|   119 +
 .../rtlwifi/halmac/halmac_fw_offload_c2h_nic.h |   371 +
 .../rtlwifi/halmac/halmac_fw_offload_h2c_nic.h |   694 +