| Message ID | 20230715112523.2533742-4-contact@jookia.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add support for Allwinner D1 CAN controllers | expand |
| Context | Check | Description |
|---|---|---|
| conchuod/cover_letter | success | Series has a cover letter |
| conchuod/tree_selection | success | Guessed tree name to be for-next at HEAD 471aba2e4760 |
| conchuod/fixes_present | success | Fixes tag not required for -next series |
| conchuod/maintainers_pattern | success | MAINTAINERS pattern errors before the patch: 4 and now 4 |
| conchuod/verify_signedoff | success | Signed-off-by tag matches author and committer |
| conchuod/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| conchuod/build_rv64_clang_allmodconfig | success | Errors and warnings before: 9 this patch: 9 |
| conchuod/module_param | success | Was 0 now: 0 |
| conchuod/build_rv64_gcc_allmodconfig | success | Errors and warnings before: 9 this patch: 9 |
| conchuod/build_rv32_defconfig | success | Build OK |
| conchuod/dtb_warn_rv64 | success | Errors and warnings before: 3 this patch: 3 |
| conchuod/header_inline | success | No static functions without inline keyword in header files |
| conchuod/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 36 lines checked |
| conchuod/build_rv64_nommu_k210_defconfig | success | Build OK |
| conchuod/verify_fixes | success | No Fixes tag |
| conchuod/build_rv64_nommu_virt_defconfig | success | Build OK |
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
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.
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
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
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
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.
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)");