mbox series

[v2,0/3] Allwinner R329 {R-,}PIO pinctrl support

Message ID 20220710081853.1699028-1-uwu@icenowy.me
Headers show
Series Allwinner R329 {R-,}PIO pinctrl support | expand

Message

Icenowy Zheng July 10, 2022, 8:18 a.m. UTC 
This patchset contains support for two pin controllers on Allwinner R329
one, a CPUX one and a CPUS one (the standby processor on R329 is, in
fact, a Xtensa DSP), in addition to their bindings.

Icenowy Zheng (3):
  dt-bindings: pinctrl: document Allwinner R329 PIO and R-PIO
  pinctrl: sunxi: add support for R329 CPUX pin controller
  pinctrl: sunxi: add support for R329 R-PIO pin controller

 .../pinctrl/allwinner,sun4i-a10-pinctrl.yaml  |   4 +
 drivers/pinctrl/sunxi/Kconfig                 |  10 +
 drivers/pinctrl/sunxi/Makefile                |   2 +
 drivers/pinctrl/sunxi/pinctrl-sun50i-r329-r.c | 293 +++++++++++++
 drivers/pinctrl/sunxi/pinctrl-sun50i-r329.c   | 412 ++++++++++++++++++
 5 files changed, 721 insertions(+)
 create mode 100644 drivers/pinctrl/sunxi/pinctrl-sun50i-r329-r.c
 create mode 100644 drivers/pinctrl/sunxi/pinctrl-sun50i-r329.c

Comments

Icenowy Zheng July 12, 2022, 9:37 a.m. UTC | #1 
在 2022-07-10星期日的 21:06 +0200，Andy Shevchenko写道：
> On Sun, Jul 10, 2022 at 10:22 AM Icenowy Zheng <uwu@icenowy.me>
> wrote:
> > 
> > Allwinner R329 SoC has two pin controllers similar to ones on
> > previous
> > SoCs, one in CPUX power domain and another in CPUS.
> > 
> > This patch adds support for the CPUX domain pin controller.
> 
> ...
> 
> > +#include <linux/module.h>
> > +#include <linux/platform_device.h>
> 
> > +#include <linux/of.h>
> > +#include <linux/of_device.h>
> 
> No use of these.
> 
> > +#include <linux/pinctrl/pinctrl.h>
> 
> Missed headers:
> mod_devicetable.h

Thanks for these.

In addition, how to decide what header should be included? The code
works properly because of_device.h includes mod_devicetable.h.

> 
> > +#include "pinctrl-sunxi.h"
>
Andy Shevchenko July 12, 2022, 10:11 a.m. UTC | #2 
On Tue, Jul 12, 2022 at 12:10 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
>
> On Tue, Jul 12, 2022 at 11:37 AM Icenowy Zheng <uwu@icenowy.me> wrote:
> > 在 2022-07-10星期日的 21:06 +0200，Andy Shevchenko写道：
> > > On Sun, Jul 10, 2022 at 10:22 AM Icenowy Zheng <uwu@icenowy.me>
> > > wrote:
>
> ...
>
> > > > +#include <linux/of.h>
> > > > +#include <linux/of_device.h>
> > >
> > > No use of these.
> > >
> > > > +#include <linux/pinctrl/pinctrl.h>
> > >
> > > Missed headers:
> > > mod_devicetable.h
> >
> > Thanks for these.
> >
> > In addition, how to decide what header should be included? The code
> > works properly because of_device.h includes mod_devicetable.h.

Forgot to put in the first place this:

The rule of thumb is Include headers that the header or C module is
direct user of. Additional information is located in the sections
below.

> The best approach is usually learnt with experience — the more you
> code, the more you get. One of the rules is to avoid too many
> inclusions and at the same time reduce indirect inclusions, so that
> the headers which are guaranteed to be included by others shouldn't
> appear. Another rule is that, for the headers (*.h files) the forward
> declarations are preferable over the inclusion in case if the opaque
> pointers are in use.