[PATCH 0/4] Add support for Allwinner D1 CAN controllers

[PATCH 0/4] Add support for Allwinner D1 CAN controllers

[Jookia]

From: John Watts <contact@jookia.org>

This patch series adds support for the Allwinner D1 CAN controllers.
It requires adding a new device tree compatible and driver support to
work around some hardware quirks.

John Watts (4):
  dt-bindings: net: can: Add support for Allwinner D1 CAN controller
  riscv: dts: allwinner: d1: Add CAN controller nodes
  can: sun4i_can: Add send support for the Allwinner D1
  can: sun4i_can: Correctly set acceptance registers on the D1

 .../net/can/allwinner,sun4i-a10-can.yaml      |  6 ++--
 .../boot/dts/allwinner/sunxi-d1s-t113.dtsi    | 30 +++++++++++++++++++
 drivers/net/can/Kconfig                       |  4 +--
 drivers/net/can/sun4i_can.c                   | 22 ++++++++++++--
 4 files changed, 55 insertions(+), 7 deletions(-)

-- 
2.41.0

[PATCH 1/4] dt-bindings: net: can: Add support for Allwinner D1 CAN controller

[Jookia]

From: John Watts <contact@jookia.org>

The Allwinner D1 has two CAN controllers, both a variant of the R40
controller. Unfortunately the registers for the D1 controllers are
moved around enough to be incompatible and require a new compatible.

Introduce the "allwinner,sun20i-d1-can" compatible to support this.

Signed-off-by: John Watts <contact@jookia.org>
---
 .../bindings/net/can/allwinner,sun4i-a10-can.yaml           | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/can/allwinner,sun4i-a10-can.yaml b/Documentation/devicetree/bindings/net/can/allwinner,sun4i-a10-can.yaml
index 9c494957a07a..e42ea28d6ab4 100644
--- a/Documentation/devicetree/bindings/net/can/allwinner,sun4i-a10-can.yaml
+++ b/Documentation/devicetree/bindings/net/can/allwinner,sun4i-a10-can.yaml
@@ -21,6 +21,7 @@ properties:
           - const: allwinner,sun4i-a10-can
       - const: allwinner,sun4i-a10-can
       - const: allwinner,sun8i-r40-can
+      - const: allwinner,sun20i-d1-can
 
   reg:
     maxItems: 1
@@ -37,8 +38,9 @@ properties:
 if:
   properties:
     compatible:
-      contains:
-        const: allwinner,sun8i-r40-can
+      enum:
+        - allwinner,sun8i-r40-can
+        - allwinner,sun20i-d1-can
 
 then:
   required:
-- 
2.41.0

[PATCH 2/4] riscv: dts: allwinner: d1: Add CAN controller nodes

[Jookia]

From: John Watts <contact@jookia.org>

The Allwinner D1, T113 provide two CAN controllers that are variants
of the R40 controller.

I have tested support for these controllers on two boards:

- A Lichee Panel RV 86 Panel running a D1 chip
- A Mango Pi MQ Dual running a T113-s3 chip

Both of these fully support both CAN controllers.

Signed-off-by: John Watts <contact@jookia.org>
---
 .../boot/dts/allwinner/sunxi-d1s-t113.dtsi    | 30 +++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/arch/riscv/boot/dts/allwinner/sunxi-d1s-t113.dtsi b/arch/riscv/boot/dts/allwinner/sunxi-d1s-t113.dtsi
index 1bb1e5cae602..b185398334be 100644
--- a/arch/riscv/boot/dts/allwinner/sunxi-d1s-t113.dtsi
+++ b/arch/riscv/boot/dts/allwinner/sunxi-d1s-t113.dtsi
@@ -131,6 +131,18 @@ uart3_pb_pins: uart3-pb-pins {
 				pins = "PB6", "PB7";
 				function = "uart3";
 			};
+
+			/omit-if-no-ref/
+			can0_pins: can0_pins {
+				pins = "PB2", "PB3";
+				function = "can0";
+			};
+
+			/omit-if-no-ref/
+			can1_pins: can1_pins {
+				pins = "PB4", "PB5";
+				function = "can1";
+			};
 		};
 
 		ccu: clock-controller@2001000 {
@@ -879,5 +891,23 @@ rtc: rtc@7090000 {
 			clock-names = "bus", "hosc", "ahb";
 			#clock-cells = <1>;
 		};
+
+		can0: can@2504000 {
+			compatible = "allwinner,sun20i-d1-can";
+			reg = <0x02504000 0x400>;
+			interrupts = <SOC_PERIPHERAL_IRQ(21) IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&ccu CLK_BUS_CAN0>;
+			resets = <&ccu RST_BUS_CAN0>;
+			status = "disabled";
+		};
+
+		can1: can@2504400 {
+			compatible = "allwinner,sun20i-d1-can";
+			reg = <0x02504400 0x400>;
+			interrupts = <SOC_PERIPHERAL_IRQ(22) IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&ccu CLK_BUS_CAN1>;
+			resets = <&ccu RST_BUS_CAN1>;
+			status = "disabled";
+		};
 	};
 };
-- 
2.41.0

Re: [PATCH 2/4] riscv: dts: allwinner: d1: Add CAN controller nodes

[Krzysztof Kozlowski]

On 15/07/2023 13:25, Jookia wrote:
> From: John Watts <contact@jookia.org>
> 
> The Allwinner D1, T113 provide two CAN controllers that are variants
> of the R40 controller.
> 
> I have tested support for these controllers on two boards:
> 

Please use scripts/get_maintainers.pl to get a list of necessary people
and lists to CC (and consider --no-git-fallback argument). It might
happen, that command when run on an older kernel, gives you outdated
entries. Therefore please be sure you base your patches on recent Linux
kernel.

> - A Lichee Panel RV 86 Panel running a D1 chip
> - A Mango Pi MQ Dual running a T113-s3 chip
> 
> Both of these fully support both CAN controllers.
> 
> Signed-off-by: John Watts <contact@jookia.org>
> ---
>  .../boot/dts/allwinner/sunxi-d1s-t113.dtsi    | 30 +++++++++++++++++++
>  1 file changed, 30 insertions(+)
> 
> diff --git a/arch/riscv/boot/dts/allwinner/sunxi-d1s-t113.dtsi b/arch/riscv/boot/dts/allwinner/sunxi-d1s-t113.dtsi
> index 1bb1e5cae602..b185398334be 100644
> --- a/arch/riscv/boot/dts/allwinner/sunxi-d1s-t113.dtsi
> +++ b/arch/riscv/boot/dts/allwinner/sunxi-d1s-t113.dtsi
> @@ -131,6 +131,18 @@ uart3_pb_pins: uart3-pb-pins {
>  				pins = "PB6", "PB7";
>  				function = "uart3";
>  			};
> +
> +			/omit-if-no-ref/
> +			can0_pins: can0_pins {

Wrong node naming. Underscores are not allowed.

It does not look like you tested the DTS against bindings. Please run
`make dtbs_check` (see
Documentation/devicetree/bindings/writing-schema.rst or
https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/
for instructions).



Best regards,
Krzysztof

Re: [PATCH 2/4] riscv: dts: allwinner: d1: Add CAN controller nodes

[John Watts]

Hello,

Thanks for spending your time replying to this.

On Sun, Jul 16, 2023 at 06:35:17PM +0200, Krzysztof Kozlowski wrote:
> On 15/07/2023 13:25, Jookia wrote:
> > From: John Watts <contact@jookia.org>
> > 
> > The Allwinner D1, T113 provide two CAN controllers that are variants
> > of the R40 controller.
> > 
> > I have tested support for these controllers on two boards:
> > 
> 
> Please use scripts/get_maintainers.pl to get a list of necessary people
> and lists to CC (and consider --no-git-fallback argument). It might
> happen, that command when run on an older kernel, gives you outdated
> entries. Therefore please be sure you base your patches on recent Linux
> kernel.

I ran this script and selected some of the emails and CCed them, at least I
believe that's what I did. Maybe I lost them when copying them to vim to make
the command line arguments.

> > +
> > +			/omit-if-no-ref/
> > +			can0_pins: can0_pins {
> 
> Wrong node naming. Underscores are not allowed.
> 
> It does not look like you tested the DTS against bindings. Please run
> `make dtbs_check` (see
> Documentation/devicetree/bindings/writing-schema.rst or
> https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/
> for instructions).

Hmm. I spent a few hours struggling and testing with make dtbs_check and
dt_binding_check. It doesn't seem to pick up on this specific issue, or at
least not in this case.

But yes I do see this, thanks for the catch. Will fix in v2.

I'm basing this patch series on 6.5-rc1, does that make a difference?

> Best regards,
> Krzysztof

Thanks,
John.

Re: [PATCH 2/4] riscv: dts: allwinner: d1: Add CAN controller nodes

[Krzysztof Kozlowski]

On 16/07/2023 18:50, John Watts wrote:
> Hello,
> 
> Thanks for spending your time replying to this.
> 
> On Sun, Jul 16, 2023 at 06:35:17PM +0200, Krzysztof Kozlowski wrote:
>> On 15/07/2023 13:25, Jookia wrote:
>>> From: John Watts <contact@jookia.org>
>>>
>>> The Allwinner D1, T113 provide two CAN controllers that are variants
>>> of the R40 controller.
>>>
>>> I have tested support for these controllers on two boards:
>>>
>>
>> Please use scripts/get_maintainers.pl to get a list of necessary people
>> and lists to CC (and consider --no-git-fallback argument). It might
>> happen, that command when run on an older kernel, gives you outdated
>> entries. Therefore please be sure you base your patches on recent Linux
>> kernel.
> 
> I ran this script and selected some of the emails and CCed them, at least I

So the others you skipped  Like let's pick this maintainer, but skip the
others? This is not how does it work. You are expected to CC all
maintainers (and to clarify the obvious: maintainers, not random
contributors). Some maintainers explicitly agree to be skipped, but
where do you see such statement from me?

Best regards,
Krzysztof

Re: [PATCH 2/4] riscv: dts: allwinner: d1: Add CAN controller nodes

[John Watts]

On Mon, Jul 17, 2023 at 08:39:52AM +0200, Krzysztof Kozlowski wrote:
> So the others you skipped  Like let's pick this maintainer, but skip the
> others? This is not how does it work. You are expected to CC all
> maintainers (and to clarify the obvious: maintainers, not random
> contributors). Some maintainers explicitly agree to be skipped, but
> where do you see such statement from me?

Apologies, I didn't know you were supposed to CC all maintainers in the list.

> 
> Best regards,
> Krzysztof
> 

John.

[PATCH 3/4] can: sun4i_can: Add send support for the Allwinner D1

[Jookia]

From: John Watts <contact@jookia.org>

The controllers present in the D1 are extremely similar to the R40
and require the same reset quirks. This alone can support sending
packets. An extra quirk is needed to support receiving packets.

Signed-off-by: John Watts <contact@jookia.org>
---
 drivers/net/can/Kconfig     | 4 ++--
 drivers/net/can/sun4i_can.c | 9 ++++++++-
 2 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/net/can/Kconfig b/drivers/net/can/Kconfig
index a5c5036dfb94..e626de33e735 100644
--- a/drivers/net/can/Kconfig
+++ b/drivers/net/can/Kconfig
@@ -185,10 +185,10 @@ config CAN_SLCAN
 
 config CAN_SUN4I
 	tristate "Allwinner A10 CAN controller"
-	depends on MACH_SUN4I || MACH_SUN7I || COMPILE_TEST
+	depends on MACH_SUN4I || MACH_SUN7I || RISCV || COMPILE_TEST
 	help
 	  Say Y here if you want to use CAN controller found on Allwinner
-	  A10/A20 SoCs.
+	  A10/A20/D1 SoCs.
 
 	  To compile this driver as a module, choose M here: the module will
 	  be called sun4i_can.
diff --git a/drivers/net/can/sun4i_can.c b/drivers/net/can/sun4i_can.c
index 0827830bbf28..06f2cf05aaf5 100644
--- a/drivers/net/can/sun4i_can.c
+++ b/drivers/net/can/sun4i_can.c
@@ -774,6 +774,10 @@ static const struct sun4ican_quirks sun4ican_quirks_r40 = {
 	.has_reset = true,
 };
 
+static const struct sun4ican_quirks sun4ican_quirks_d1 = {
+	.has_reset = true,
+};
+
 static const struct of_device_id sun4ican_of_match[] = {
 	{
 		.compatible = "allwinner,sun4i-a10-can",
@@ -784,6 +788,9 @@ static const struct of_device_id sun4ican_of_match[] = {
 	}, {
 		.compatible = "allwinner,sun8i-r40-can",
 		.data = &sun4ican_quirks_r40
+	}, {
+		.compatible = "allwinner,sun20i-d1-can",
+		.data = &sun4ican_quirks_d1
 	}, {
 		/* sentinel */
 	},
@@ -907,4 +914,4 @@ module_platform_driver(sun4i_can_driver);
 MODULE_AUTHOR("Peter Chen <xingkongcp@gmail.com>");
 MODULE_AUTHOR("Gerhard Bertelsmann <info@gerhard-bertelsmann.de>");
 MODULE_LICENSE("Dual BSD/GPL");
-MODULE_DESCRIPTION("CAN driver for Allwinner SoCs (A10/A20)");
+MODULE_DESCRIPTION("CAN driver for Allwinner SoCs (A10/A20/D1)");
-- 
2.41.0

Re: [PATCH 3/4] can: sun4i_can: Add send support for the Allwinner D1

[Krzysztof Kozlowski]

On 15/07/2023 13:25, Jookia wrote:
> From: John Watts <contact@jookia.org>
> 
> The controllers present in the D1 are extremely similar to the R40
> and require the same reset quirks. This alone can support sending
> packets. An extra quirk is needed to support receiving packets.
> 
> Signed-off-by: John Watts <contact@jookia.org>
> ---
>  drivers/net/can/Kconfig     | 4 ++--
>  drivers/net/can/sun4i_can.c | 9 ++++++++-
>  2 files changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/can/Kconfig b/drivers/net/can/Kconfig
> index a5c5036dfb94..e626de33e735 100644
> --- a/drivers/net/can/Kconfig
> +++ b/drivers/net/can/Kconfig
> @@ -185,10 +185,10 @@ config CAN_SLCAN
>  
>  config CAN_SUN4I
>  	tristate "Allwinner A10 CAN controller"
> -	depends on MACH_SUN4I || MACH_SUN7I || COMPILE_TEST
> +	depends on MACH_SUN4I || MACH_SUN7I || RISCV || COMPILE_TEST
>  	help
>  	  Say Y here if you want to use CAN controller found on Allwinner
> -	  A10/A20 SoCs.
> +	  A10/A20/D1 SoCs.
>  
>  	  To compile this driver as a module, choose M here: the module will
>  	  be called sun4i_can.
> diff --git a/drivers/net/can/sun4i_can.c b/drivers/net/can/sun4i_can.c
> index 0827830bbf28..06f2cf05aaf5 100644
> --- a/drivers/net/can/sun4i_can.c
> +++ b/drivers/net/can/sun4i_can.c
> @@ -774,6 +774,10 @@ static const struct sun4ican_quirks sun4ican_quirks_r40 = {
>  	.has_reset = true,
>  };
>  
> +static const struct sun4ican_quirks sun4ican_quirks_d1 = {
> +	.has_reset = true,
> +};

Isn't this the same as previous?

Best regards,
Krzysztof

Re: [PATCH 3/4] can: sun4i_can: Add send support for the Allwinner D1

[John Watts]

Hello,

On Sun, Jul 16, 2023 at 06:36:03PM +0200, Krzysztof Kozlowski wrote:
> > +static const struct sun4ican_quirks sun4ican_quirks_d1 = {
> > +	.has_reset = true,
> > +};
> 
> Isn't this the same as previous?

Yes, but I wanted to split up the new quirk in to its own patch. Is
there a better way of doing that?

> 
> Best regards,
> Krzysztof
> 

John.

Re: [PATCH 3/4] can: sun4i_can: Add send support for the Allwinner D1

[Krzysztof Kozlowski]

On 16/07/2023 18:52, John Watts wrote:
> Hello,
> 
> On Sun, Jul 16, 2023 at 06:36:03PM +0200, Krzysztof Kozlowski wrote:
>>> +static const struct sun4ican_quirks sun4ican_quirks_d1 = {
>>> +	.has_reset = true,
>>> +};
>>
>> Isn't this the same as previous?
> 
> Yes, but I wanted to split up the new quirk in to its own patch.

I don't understand why you need this new, duplicated entry. Aren't
devices compatible?


I also do not understand what 'own patch' has anything to do with it.

> Is
> there a better way of doing that?

Devices are compatible, right? So express it in the bindings and DTS.


Best regards,
Krzysztof

Re: [PATCH 3/4] can: sun4i_can: Add send support for the Allwinner D1

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 1411 bytes --]

On 17.07.2023 08:41:07, Krzysztof Kozlowski wrote:
> On 16/07/2023 18:52, John Watts wrote:
> > Hello,
> > 
> > On Sun, Jul 16, 2023 at 06:36:03PM +0200, Krzysztof Kozlowski wrote:
> >>> +static const struct sun4ican_quirks sun4ican_quirks_d1 = {
> >>> +	.has_reset = true,
> >>> +};
> >>
> >> Isn't this the same as previous?
> > 
> > Yes, but I wanted to split up the new quirk in to its own patch.
> 
> I don't understand why you need this new, duplicated entry. Aren't
> devices compatible?

According to patch 4/4 the devices are not compatible.

> I also do not understand what 'own patch' has anything to do with it.

Patch 4/4 adds a new quirk to the new device.

Jookia, please let the patches build on each other so that the resulting
kernel is consistent. The kernel at the state 3/4 will build and load
the driver on the D1, but it will not work, as the new quirk is missing.
Please exchange patches 3/4 and 4/4 (add the sun4ican_quirks_d1 in patch
4/4 only).

> > Is
> > there a better way of doing that?
> 
> Devices are compatible, right? So express it in the bindings and DTS.

regards,
Marc

-- 
Pengutronix e.K.                 | Marc Kleine-Budde          |
Embedded Linux                   | https://www.pengutronix.de |
Vertretung Nürnberg              | Phone: +49-5121-206917-129 |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-9   |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 3/4] can: sun4i_can: Add send support for the Allwinner D1

[Krzysztof Kozlowski]

On 17/07/2023 09:03, Marc Kleine-Budde wrote:
> On 17.07.2023 08:41:07, Krzysztof Kozlowski wrote:
>> On 16/07/2023 18:52, John Watts wrote:
>>> Hello,
>>>
>>> On Sun, Jul 16, 2023 at 06:36:03PM +0200, Krzysztof Kozlowski wrote:
>>>>> +static const struct sun4ican_quirks sun4ican_quirks_d1 = {
>>>>> +	.has_reset = true,
>>>>> +};
>>>>
>>>> Isn't this the same as previous?
>>>
>>> Yes, but I wanted to split up the new quirk in to its own patch.
>>
>> I don't understand why you need this new, duplicated entry. Aren't
>> devices compatible?
> 
> According to patch 4/4 the devices are not compatible.

Ah, ok, I didn't go so far because it is not obvious to add support for
a device in patch 3 which is already not correct and needs fix/followup
in patch 4.

Thanks.
> 

Best regards,
Krzysztof

Re: [PATCH 3/4] can: sun4i_can: Add send support for the Allwinner D1

[John Watts]

On Mon, Jul 17, 2023 at 09:03:06AM +0200, Marc Kleine-Budde wrote:
> Jookia, please let the patches build on each other so that the resulting
> kernel is consistent. The kernel at the state 3/4 will build and load
> the driver on the D1, but it will not work, as the new quirk is missing.
> Please exchange patches 3/4 and 4/4 (add the sun4ican_quirks_d1 in patch
> 4/4 only).

Shall do, thank you.

> regards,
> Marc
> 
> -- 
> Pengutronix e.K.                 | Marc Kleine-Budde          |
> Embedded Linux                   | https://www.pengutronix.de |
> Vertretung Nürnberg              | Phone: +49-5121-206917-129 |
> Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-9   |

John.

[PATCH 4/4] can: sun4i_can: Correctly set acceptance registers on the D1

[Jookia]

From: John Watts <contact@jookia.org>

The Allwinner D1's CAN controllers have the ACPC and ACPM registers
moved down. Compensate for this by adding an offset quirk for the
acceptance registers.

Signed-off-by: John Watts <contact@jookia.org>
---
 drivers/net/can/sun4i_can.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/net/can/sun4i_can.c b/drivers/net/can/sun4i_can.c
index 06f2cf05aaf5..c508a328e38d 100644
--- a/drivers/net/can/sun4i_can.c
+++ b/drivers/net/can/sun4i_can.c
@@ -91,6 +91,8 @@
 #define SUN4I_REG_BUF12_ADDR	0x0070	/* CAN Tx/Rx Buffer 12 */
 #define SUN4I_REG_ACPC_ADDR	0x0040	/* CAN Acceptance Code 0 */
 #define SUN4I_REG_ACPM_ADDR	0x0044	/* CAN Acceptance Mask 0 */
+#define SUN4I_REG_ACPC_ADDR_D1	0x0028	/* CAN Acceptance Code 0 on the D1 */
+#define SUN4I_REG_ACPM_ADDR_D1	0x002C	/* CAN Acceptance Mask 0 on the D1 */
 #define SUN4I_REG_RBUF_RBACK_START_ADDR	0x0180	/* CAN transmit buffer start */
 #define SUN4I_REG_RBUF_RBACK_END_ADDR	0x01b0	/* CAN transmit buffer end */
 
@@ -205,9 +207,11 @@
  * struct sun4ican_quirks - Differences between SoC variants.
  *
  * @has_reset: SoC needs reset deasserted.
+ * @acp_offset: Offset of ACPC and ACPM registers
  */
 struct sun4ican_quirks {
 	bool has_reset;
+	int acp_offset;
 };
 
 struct sun4ican_priv {
@@ -216,6 +220,7 @@ struct sun4ican_priv {
 	struct clk *clk;
 	struct reset_control *reset;
 	spinlock_t cmdreg_lock;	/* lock for concurrent cmd register writes */
+	int acp_offset;
 };
 
 static const struct can_bittiming_const sun4ican_bittiming_const = {
@@ -338,8 +343,8 @@ static int sun4i_can_start(struct net_device *dev)
 	}
 
 	/* set filters - we accept all */
-	writel(0x00000000, priv->base + SUN4I_REG_ACPC_ADDR);
-	writel(0xFFFFFFFF, priv->base + SUN4I_REG_ACPM_ADDR);
+	writel(0x00000000, priv->base + SUN4I_REG_ACPC_ADDR + priv->acp_offset);
+	writel(0xFFFFFFFF, priv->base + SUN4I_REG_ACPM_ADDR + priv->acp_offset);
 
 	/* clear error counters and error code capture */
 	writel(0, priv->base + SUN4I_REG_ERRC_ADDR);
@@ -768,14 +773,17 @@ static const struct ethtool_ops sun4ican_ethtool_ops = {
 
 static const struct sun4ican_quirks sun4ican_quirks_a10 = {
 	.has_reset = false,
+	.acp_offset = 0,
 };
 
 static const struct sun4ican_quirks sun4ican_quirks_r40 = {
 	.has_reset = true,
+	.acp_offset = 0,
 };
 
 static const struct sun4ican_quirks sun4ican_quirks_d1 = {
 	.has_reset = true,
+	.acp_offset = (SUN4I_REG_ACPC_ADDR_D1 - SUN4I_REG_ACPC_ADDR),
 };
 
 static const struct of_device_id sun4ican_of_match[] = {
@@ -877,6 +885,7 @@ static int sun4ican_probe(struct platform_device *pdev)
 	priv->base = addr;
 	priv->clk = clk;
 	priv->reset = reset;
+	priv->acp_offset = quirks->acp_offset;
 	spin_lock_init(&priv->cmdreg_lock);
 
 	platform_set_drvdata(pdev, dev);
-- 
2.41.0

Re: [PATCH 4/4] can: sun4i_can: Correctly set acceptance registers on the D1

[Jernej Škrabec]

Dne sobota, 15. julij 2023 ob 13:25:22 CEST je Jookia napisal(a):
> From: John Watts <contact@jookia.org>
> 
> The Allwinner D1's CAN controllers have the ACPC and ACPM registers
> moved down. Compensate for this by adding an offset quirk for the
> acceptance registers.
> 
> Signed-off-by: John Watts <contact@jookia.org>

This patch should precede patch 3, so in next patch you add full D1 support.

Best regards,
Jernej

> ---
>  drivers/net/can/sun4i_can.c | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/can/sun4i_can.c b/drivers/net/can/sun4i_can.c
> index 06f2cf05aaf5..c508a328e38d 100644
> --- a/drivers/net/can/sun4i_can.c
> +++ b/drivers/net/can/sun4i_can.c
> @@ -91,6 +91,8 @@
>  #define SUN4I_REG_BUF12_ADDR	0x0070	/* CAN Tx/Rx Buffer 12 */
>  #define SUN4I_REG_ACPC_ADDR	0x0040	/* CAN Acceptance Code 0 */
>  #define SUN4I_REG_ACPM_ADDR	0x0044	/* CAN Acceptance Mask 0 */
> +#define SUN4I_REG_ACPC_ADDR_D1	0x0028	/* CAN Acceptance Code 0 on the D1 
*/
> +#define SUN4I_REG_ACPM_ADDR_D1	0x002C	/* CAN Acceptance Mask 0 
on the D1
> */ #define SUN4I_REG_RBUF_RBACK_START_ADDR	0x0180	/* CAN transmit buffer
> start */ #define SUN4I_REG_RBUF_RBACK_END_ADDR	0x01b0	/* CAN 
transmit
> buffer end */
> 
> @@ -205,9 +207,11 @@
>   * struct sun4ican_quirks - Differences between SoC variants.
>   *
>   * @has_reset: SoC needs reset deasserted.
> + * @acp_offset: Offset of ACPC and ACPM registers
>   */
>  struct sun4ican_quirks {
>  	bool has_reset;
> +	int acp_offset;
>  };
> 
>  struct sun4ican_priv {
> @@ -216,6 +220,7 @@ struct sun4ican_priv {
>  	struct clk *clk;
>  	struct reset_control *reset;
>  	spinlock_t cmdreg_lock;	/* lock for concurrent cmd register 
writes */
> +	int acp_offset;
>  };
> 
>  static const struct can_bittiming_const sun4ican_bittiming_const = {
> @@ -338,8 +343,8 @@ static int sun4i_can_start(struct net_device *dev)
>  	}
> 
>  	/* set filters - we accept all */
> -	writel(0x00000000, priv->base + SUN4I_REG_ACPC_ADDR);
> -	writel(0xFFFFFFFF, priv->base + SUN4I_REG_ACPM_ADDR);
> +	writel(0x00000000, priv->base + SUN4I_REG_ACPC_ADDR + priv-
>acp_offset);
> +	writel(0xFFFFFFFF, priv->base + SUN4I_REG_ACPM_ADDR + priv-
>acp_offset);
> 
>  	/* clear error counters and error code capture */
>  	writel(0, priv->base + SUN4I_REG_ERRC_ADDR);
> @@ -768,14 +773,17 @@ static const struct ethtool_ops sun4ican_ethtool_ops =
> {
> 
>  static const struct sun4ican_quirks sun4ican_quirks_a10 = {
>  	.has_reset = false,
> +	.acp_offset = 0,
>  };
> 
>  static const struct sun4ican_quirks sun4ican_quirks_r40 = {
>  	.has_reset = true,
> +	.acp_offset = 0,
>  };
> 
>  static const struct sun4ican_quirks sun4ican_quirks_d1 = {
>  	.has_reset = true,
> +	.acp_offset = (SUN4I_REG_ACPC_ADDR_D1 - SUN4I_REG_ACPC_ADDR),
>  };
> 
>  static const struct of_device_id sun4ican_of_match[] = {
> @@ -877,6 +885,7 @@ static int sun4ican_probe(struct platform_device *pdev)
>  	priv->base = addr;
>  	priv->clk = clk;
>  	priv->reset = reset;
> +	priv->acp_offset = quirks->acp_offset;
>  	spin_lock_init(&priv->cmdreg_lock);
> 
>  	platform_set_drvdata(pdev, dev);

Re: [PATCH 4/4] can: sun4i_can: Correctly set acceptance registers on the D1

[John Watts]

On Sun, Jul 16, 2023 at 06:45:25PM +0200, Jernej Škrabec wrote:
> This patch should precede patch 3, so in next patch you add full D1 support.
> 
> Best regards,
> Jernej

That makes sense, thank you. Will change in v2.

John.

Re: [PATCH 0/4] Add support for Allwinner D1 CAN controllers

[Ben Dooks]

On 15/07/2023 12:25, Jookia wrote:
> From: John Watts <contact@jookia.org>

It would be better if you sent your sending email to be the same
as the from, I think you'd technically need a signed-off-by from
"Jookia" as well.

-- 
Ben Dooks				http://www.codethink.co.uk/
Senior Engineer				Codethink - Providing Genius

https://www.codethink.co.uk/privacy.html

Re: [PATCH 0/4] Add support for Allwinner D1 CAN controllers

[John Watts]

On Mon, Jul 17, 2023 at 09:00:47AM +0100, Ben Dooks wrote:
> On 15/07/2023 12:25, Jookia wrote:
> > From: John Watts <contact@jookia.org>
> 
> It would be better if you sent your sending email to be the same
> as the from, I think you'd technically need a signed-off-by from
> "Jookia" as well.

Yes, this was a msmtp mistake. Will be fixed in v2.

> 
> -- 
> Ben Dooks				http://www.codethink.co.uk/
> Senior Engineer				Codethink - Providing Genius
> 
> https://www.codethink.co.uk/privacy.html
> 

John.

Re: [PATCH 0/4] Add support for Allwinner D1 CAN controllers

[Rob Herring]

On Mon, Jul 17, 2023 at 09:00:47AM +0100, Ben Dooks wrote:
> On 15/07/2023 12:25, Jookia wrote:
> > From: John Watts <contact@jookia.org>
> 
> It would be better if you sent your sending email to be the same
> as the from, I think you'd technically need a signed-off-by from
> "Jookia" as well.

The email needs to be an actual person. 'contact' doesn't appear to be 
one.

Rob

Re: [PATCH 0/4] Add support for Allwinner D1 CAN controllers

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 655 bytes --]

On Tue, Jul 18, 2023 at 04:15:04PM -0600, Rob Herring wrote:
> On Mon, Jul 17, 2023 at 09:00:47AM +0100, Ben Dooks wrote:
> > On 15/07/2023 12:25, Jookia wrote:
> > > From: John Watts <contact@jookia.org>
> > 
> > It would be better if you sent your sending email to be the same
> > as the from, I think you'd technically need a signed-off-by from
> > "Jookia" as well.
> 
> The email needs to be an actual person. 'contact' doesn't appear to be 
> one.

I don't think it is that complicated, a cursory check of the domain
suggests that "Jookia" is a nickname & there's surely nothing wrong with
not having your name in your email address.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 0/4] Add support for Allwinner D1 CAN controllers

[John Watts]

On Tue, Jul 18, 2023 at 11:38:48PM +0100, Conor Dooley wrote:
> > The email needs to be an actual person. 'contact' doesn't appear to be 
> > one.
> 
> I don't think it is that complicated, a cursory check of the domain
> suggests that "Jookia" is a nickname & there's surely nothing wrong with
> not having your name in your email address.

Yes, this is the case. I figured contact@jookia.org was better than
jookia@jookia.org.

I generally go by my nickname online and in the open source community so
it's much more useful as an identifier in this case, but I've put my real
name here instead to make things easier on everyone.

John.