mbox series

[v2,0/6] Allwinner V3 SL631 Action Camera Support and Related Fixes

Message ID 20201103205058.435207-1-contact@paulk.fr
Headers show
Series Allwinner V3 SL631 Action Camera Support and Related Fixes | expand

Message

Paul Kocialkowski Nov. 3, 2020, 8:50 p.m. UTC 
This series adds support for the Allwinner V3-based SL631 family of
Action Cameras, starting with the IMX179 fashion.

A few fixes to V3 support are added along the way, most notably support
for the NMI IRQ controller which is necessary for the AXP209 IRQ.

Note that some patches in this series may have already been submitted
(but not yet merged) by others and are included for the series to build.

Changes since v1:
- Rework commit log messages as requested;
- Fixed v3s nmi controller compatible order in dt bindings doc;
- Changed SL631 compatible vendor to allwinner;
- Fixed LRADC button node names;
- Removed unused LDO4 regulator node;
- Removed merged patches.

Happy reviewing!

Paul Kocialkowski (6):
  dt-bindings: irq: sun7i-nmi: Add binding documentation for the V3s NMI
  irqchip/sunxi-nmi: Add support for the V3s NMI
  ARM: dts: sun8i-v3s: Add the V3s NMI IRQ controller
  ARM: dts: sun8i: Cleanup the Pinecube AXP209 node
  dt-bindings: arm: sunxi: Add SL631 with IMX179 bindings
  ARM: dts: sun8i-v3: Add support for the SL631 Action Camera with
    IMX179

 .../devicetree/bindings/arm/sunxi.yaml        |   6 +
 .../allwinner,sun7i-a20-sc-nmi.yaml           |   1 +
 arch/arm/boot/dts/Makefile                    |   1 +
 arch/arm/boot/dts/sun8i-s3-pinecube.dts       |   8 +-
 arch/arm/boot/dts/sun8i-v3-sl631-imx179.dts   |  12 ++
 arch/arm/boot/dts/sun8i-v3-sl631.dtsi         | 138 ++++++++++++++++++
 arch/arm/boot/dts/sun8i-v3s.dtsi              |  10 +-
 drivers/irqchip/irq-sunxi-nmi.c               |  18 ++-
 8 files changed, 186 insertions(+), 8 deletions(-)
 create mode 100644 arch/arm/boot/dts/sun8i-v3-sl631-imx179.dts
 create mode 100644 arch/arm/boot/dts/sun8i-v3-sl631.dtsi

Comments

Samuel Holland Nov. 5, 2020, 4:14 a.m. UTC | #1 
On 11/3/20 2:50 PM, Paul Kocialkowski wrote:
> The V3s/V3 has a NMI IRQ controller, which is mainly used for the AXP209
> interrupt. In great wisdom, Allwinner decided to invert the enable and
> pending register offsets, compared to the A20.
> 
> As a result, a specific compatible and register description is required
> for the V3s. This was tested with an AXP209 on a V3 board.
> 
> Acked-by: Maxime Ripard <mripard@kernel.org>
> Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
> ---
>  drivers/irqchip/irq-sunxi-nmi.c | 18 +++++++++++++++++-
>  1 file changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/irqchip/irq-sunxi-nmi.c b/drivers/irqchip/irq-sunxi-nmi.c
> index a412b5d5d0fa..59e0e4612ef7 100644
> --- a/drivers/irqchip/irq-sunxi-nmi.c
> +++ b/drivers/irqchip/irq-sunxi-nmi.c
> @@ -44,6 +44,10 @@
>  #define SUN7I_NMI_PENDING	0x04
>  #define SUN7I_NMI_ENABLE	0x08
>  
> +#define SUN8I_V3S_NMI_CTRL	0x00
> +#define SUN8I_V3S_NMI_ENABLE	0x04
> +#define SUN8I_V3S_NMI_PENDING	0x08
> +
>  #define SUN9I_NMI_CTRL		0x00
>  #define SUN9I_NMI_ENABLE	0x04
>  #define SUN9I_NMI_PENDING	0x08

These two sets of definitions are the same. So it would make sense for
V3S and sun9i to share a configuration, instead of creating a copy.

> @@ -79,6 +83,12 @@ static const struct sunxi_sc_nmi_reg_offs sun7i_reg_offs __initconst = {
>  	.enable	= SUN7I_NMI_ENABLE,
>  };
>  
> +static const struct sunxi_sc_nmi_reg_offs sun8i_v3s_reg_offs __initconst = {
> +	.ctrl	= SUN8I_V3S_NMI_CTRL,
> +	.pend	= SUN8I_V3S_NMI_PENDING,
> +	.enable	= SUN8I_V3S_NMI_ENABLE,
> +};
> +
>  static const struct sunxi_sc_nmi_reg_offs sun9i_reg_offs __initconst = {
>  	.ctrl	= SUN9I_NMI_CTRL,
>  	.pend	= SUN9I_NMI_PENDING,
> @@ -165,7 +175,6 @@ static int __init sunxi_sc_nmi_irq_init(struct device_node *node,
>  	unsigned int clr = IRQ_NOREQUEST | IRQ_NOPROBE | IRQ_NOAUTOEN;
>  	int ret;
>  
> -
>  	domain = irq_domain_add_linear(node, 1, &irq_generic_chip_ops, NULL);
>  	if (!domain) {
>  		pr_err("Could not register interrupt domain.\n");
> @@ -254,6 +263,13 @@ static int __init sun7i_sc_nmi_irq_init(struct device_node *node,
>  }
>  IRQCHIP_DECLARE(sun7i_sc_nmi, "allwinner,sun7i-a20-sc-nmi", sun7i_sc_nmi_irq_init);
>  
> +static int __init sun8i_v3s_sc_nmi_irq_init(struct device_node *node,
> +					    struct device_node *parent)
> +{
> +	return sunxi_sc_nmi_irq_init(node, &sun8i_v3s_reg_offs);
> +}
> +IRQCHIP_DECLARE(sun8i_v3s_sc_nmi, "allwinner,sun8i-v3s-sc-nmi", sun8i_v3s_sc_nmi_irq_init);
> +
>  static int __init sun9i_nmi_irq_init(struct device_node *node,
>  				     struct device_node *parent)
>  {
>
Paul Kocialkowski Nov. 5, 2020, 11:24 a.m. UTC | #2 
Hi Samuel,

On Wed 04 Nov 20, 22:14, Samuel Holland wrote:
> On 11/3/20 2:50 PM, Paul Kocialkowski wrote:
> > The V3s/V3 has a NMI IRQ controller, which is mainly used for the AXP209
> > interrupt. In great wisdom, Allwinner decided to invert the enable and
> > pending register offsets, compared to the A20.
> > 
> > As a result, a specific compatible and register description is required
> > for the V3s. This was tested with an AXP209 on a V3 board.
> > 
> > Acked-by: Maxime Ripard <mripard@kernel.org>
> > Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
> > ---
> >  drivers/irqchip/irq-sunxi-nmi.c | 18 +++++++++++++++++-
> >  1 file changed, 17 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/irqchip/irq-sunxi-nmi.c b/drivers/irqchip/irq-sunxi-nmi.c
> > index a412b5d5d0fa..59e0e4612ef7 100644
> > --- a/drivers/irqchip/irq-sunxi-nmi.c
> > +++ b/drivers/irqchip/irq-sunxi-nmi.c
> > @@ -44,6 +44,10 @@
> >  #define SUN7I_NMI_PENDING	0x04
> >  #define SUN7I_NMI_ENABLE	0x08
> >  
> > +#define SUN8I_V3S_NMI_CTRL	0x00
> > +#define SUN8I_V3S_NMI_ENABLE	0x04
> > +#define SUN8I_V3S_NMI_PENDING	0x08
> > +
> >  #define SUN9I_NMI_CTRL		0x00
> >  #define SUN9I_NMI_ENABLE	0x04
> >  #define SUN9I_NMI_PENDING	0x08
> 
> These two sets of definitions are the same. So it would make sense for
> V3S and sun9i to share a configuration, instead of creating a copy.

Oh but that's true! I initially though it was the same as sun7i, found that it
wasn't but didn't notice about sun9i.

So I think we can just use the sun9i compatible after all.

Thanks!

Paul

> > @@ -79,6 +83,12 @@ static const struct sunxi_sc_nmi_reg_offs sun7i_reg_offs __initconst = {
> >  	.enable	= SUN7I_NMI_ENABLE,
> >  };
> >  
> > +static const struct sunxi_sc_nmi_reg_offs sun8i_v3s_reg_offs __initconst = {
> > +	.ctrl	= SUN8I_V3S_NMI_CTRL,
> > +	.pend	= SUN8I_V3S_NMI_PENDING,
> > +	.enable	= SUN8I_V3S_NMI_ENABLE,
> > +};
> > +
> >  static const struct sunxi_sc_nmi_reg_offs sun9i_reg_offs __initconst = {
> >  	.ctrl	= SUN9I_NMI_CTRL,
> >  	.pend	= SUN9I_NMI_PENDING,
> > @@ -165,7 +175,6 @@ static int __init sunxi_sc_nmi_irq_init(struct device_node *node,
> >  	unsigned int clr = IRQ_NOREQUEST | IRQ_NOPROBE | IRQ_NOAUTOEN;
> >  	int ret;
> >  
> > -
> >  	domain = irq_domain_add_linear(node, 1, &irq_generic_chip_ops, NULL);
> >  	if (!domain) {
> >  		pr_err("Could not register interrupt domain.\n");
> > @@ -254,6 +263,13 @@ static int __init sun7i_sc_nmi_irq_init(struct device_node *node,
> >  }
> >  IRQCHIP_DECLARE(sun7i_sc_nmi, "allwinner,sun7i-a20-sc-nmi", sun7i_sc_nmi_irq_init);
> >  
> > +static int __init sun8i_v3s_sc_nmi_irq_init(struct device_node *node,
> > +					    struct device_node *parent)
> > +{
> > +	return sunxi_sc_nmi_irq_init(node, &sun8i_v3s_reg_offs);
> > +}
> > +IRQCHIP_DECLARE(sun8i_v3s_sc_nmi, "allwinner,sun8i-v3s-sc-nmi", sun8i_v3s_sc_nmi_irq_init);
> > +
> >  static int __init sun9i_nmi_irq_init(struct device_node *node,
> >  				     struct device_node *parent)
> >  {
> > 
>