All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kalle Valo <kvalo@codeaurora.org>
To: Pkshih <pkshih@realtek.com>
Cc: "briselec\@gmail.com" <briselec@gmail.com>,
	"linux-wireless\@vger.kernel.org"
	<linux-wireless@vger.kernel.org>,
	"Larry.Finger\@lwfinger.net" <Larry.Finger@lwfinger.net>,
	"chaitanya.mgit\@gmail.com" <chaitanya.mgit@gmail.com>,
	dan.carpenter@oracle.com
Subject: New realtek 11ac driver
Date: Fri, 29 Jun 2018 11:38:22 +0300	[thread overview]
Message-ID: <87h8lmf0n5.fsf_-_@codeaurora.org> (raw)
In-Reply-To: <1528162350.2800.3.camel@realtek.com> (pkshih@realtek.com's message of "Tue, 5 Jun 2018 01:32:56 +0000")

(Was "Re: [PATCH v3 00/19] rtlwifi: halmac: Add new module halmac",
changing the title to reflect what we are discussing)

Pkshih <pkshih@realtek.com> writes:

> On Thu, 2018-05-24 at 11:27 +0300, Kalle Valo wrote:
>>=C2=A0
>> 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.
>>=C2=A0
>
> Is there a proper way to look for "someone else" you mentioned?

I don't know, I think there might a project somewhere which helps with
patch review for new people but not sure about that. Adding Dan in case
he has some ideas.

> We plan to rewrite a new driver excluding agnostic OS layer to support=C2=
=A0
> new generation 11AC chips, because they're very different from the chips
> existed in rtlwifi and rtl8xxxu.=C2=A0
>
> If we have a "someone" to review our driver, where is the proper place to
> put developing driver repository? Staging or public git repository=C2=A0
> (e.g. GitHub)?

It depends on the driver really. If it's a good (code) quality driver
following upstream rules, then taking it directly to
drivers/net/wireless is the best approach. That's what we did with
qtnfmac for example.

But if the driver is more like a usual vendor driver with horrible code,
and not following upstream rules, then other options are better. I don't
look at staging at all so I can't comment staging vs github.com, others
more knowledgeable can comment about that.

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

You mean like written rules? I don't think that I have seen anything.
But here are some things I usually check when reviewing patches for
upstream:

* good quality, simple, self-documenting and readable code (no magic
  values, ugly hacks)

* in general follows Linux coding style (not every whitespace needs to
  be correct but most of the rules need to be followed, no CamelCode etc)

* clean and simple design (no unnecessary layers and such)

* respects cfg80211 and mac80211 designs (doesn't reinvent the wheel,
  functionality which should be in mac80211 or cfg80211 is not in the
  driver)

* user space interfaces follow the rules

* commit log answers to "Why?" and doesn't leave questions unanswered

This is not a comprehensive list but hopefully still helps you to give
an idea what kind of things we are looking for. I'm sure there are more
tips elsewhere.

And as the last tip I want to give that try to submit the driver as
simple as possible (=3Dsmall), once it's accepted you can start adding
more features on top. In other words: "Submit early, submit often"

https://en.wikipedia.org/wiki/Release_early,_release_often

--=20
Kalle Valo

  reply	other threads:[~2018-06-29  8:38 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-04-25  2:08 [PATCH v3 00/19] rtlwifi: halmac: Add new module halmac pkshih
2018-04-25  2:08 ` [PATCH v3 01/19] rtlwifi: add halmac structure to wifi.h pkshih
2018-04-25  2:08 ` [PATCH v3 02/19] rtlwifi: add debug ID COMP_HALMAC pkshih
2018-04-25  2:08 ` [PATCH v3 03/19] rtlwifi: add dmdef.h to share with driver and other modules pkshih
2018-04-25  2:08 ` [PATCH v3 04/19] rtlwifi: halmac: add main definition used by halmac pkshih
2018-04-25  2:08 ` [PATCH v3 05/19] rtlwifi: halmac: describe number and size of chip functions pkshih
2018-04-25  2:08 ` [PATCH v3 06/19] rtlwifi: halmac: add register definitions pkshih
2018-04-25  2:08 ` [PATCH v3 07/19] rtlwifi: halmac: add bit field definitions pkshih
2018-04-25  2:08 ` [PATCH v3 08/19] rtlwifi: halmac: add bit field definitions of rtl8822b pkshih
2018-04-25  2:08 ` [PATCH v3 09/19] rtlwifi: halmac: add definition of TX/RX descriptor pkshih
2018-04-25  2:08 ` [PATCH v3 10/19] rtlwifi: halmac: add GPIO pin/pinmux definitions pkshih
2018-04-25  2:08 ` [PATCH v3 11/19] rtlwifi: halmac: add power sequence to turn on/off wifi card pkshih
2018-04-25  2:08 ` [PATCH v3 12/19] rtlwifi: halmac: access efuse through halmac helper functions pkshih
2018-04-25  2:08 ` [PATCH v3 13/19] rtlwifi: halmac: add files to implement halmac ops pkshih
2018-04-25  2:08 ` [PATCH v3 14/19] rtlwifi: halmac: add halmac init/deinit functions pkshih
2018-04-25  2:08 ` [PATCH v3 15/19] rtlwifi: halmac: add firmware related functions and definitions pkshih
2018-04-25  2:08 ` [PATCH v3 16/19] rtlwifi: halmac: add bus interface commands pkshih
2018-04-25  2:08 ` [PATCH v3 17/19] rtlwifi: halmac: add to control WiFi mac functions and registers pkshih
2018-04-25  2:08 ` [PATCH v3 18/19] rtlwifi: halmac: add to support BB and RF functions pkshih
2018-04-25  2:08 ` [PATCH v3 19/19] rtlwifi: add halmac to Makefile and Kconfig pkshih
2018-04-25  7:36 ` [PATCH v3 00/19] rtlwifi: halmac: Add new module halmac Kalle Valo
2018-04-27  5:44   ` Pkshih
2018-04-27 22:41     ` Barry Day
2018-04-30  2:40       ` Pkshih
2018-04-30  8:33         ` Krishna Chaitanya
2018-05-15  8:08           ` Pkshih
2018-05-16 12:36             ` Kalle Valo
2018-05-18 12:33               ` Pkshih
2018-05-24  8:27                 ` Kalle Valo
2018-06-05  1:32                   ` Pkshih
2018-06-29  8:38                     ` Kalle Valo [this message]
2018-07-02  7:21                       ` New realtek 11ac driver Dan Carpenter
2018-05-16 12:01           ` [PATCH v3 00/19] rtlwifi: halmac: Add new module halmac Kalle Valo
2018-05-16 12:09     ` Kalle Valo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87h8lmf0n5.fsf_-_@codeaurora.org \
    --to=kvalo@codeaurora.org \
    --cc=Larry.Finger@lwfinger.net \
    --cc=briselec@gmail.com \
    --cc=chaitanya.mgit@gmail.com \
    --cc=dan.carpenter@oracle.com \
    --cc=linux-wireless@vger.kernel.org \
    --cc=pkshih@realtek.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.