| Message ID | 20230113084516.31888-1-lukas.bulwahn@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | PCI: rcar: avoid defines prefixed with CONFIG | expand |
Hi Lukas, On Fri, Jan 13, 2023 at 9:52 AM Lukas Bulwahn <lukas.bulwahn@gmail.com> wrote: > Defines prefixed with "CONFIG" should be limited to proper Kconfig options, > that are introduced in a Kconfig file. > > Here, a definition for a bitmask to configure the SEND_ENABLE mode is named > CONFIG_SEND_ENABLE. > > Rename this local definition to CONFIGURE_SEND_ENABLE to avoid defines > prefixed with "CONFIG". > > No functional change. > > Signed-off-by: Lukas Bulwahn <lukas.bulwahn@gmail.com> Thanks for your patch! > --- a/drivers/pci/controller/pcie-rcar.h > +++ b/drivers/pci/controller/pcie-rcar.h > @@ -11,7 +11,7 @@ > > #define PCIECAR 0x000010 > #define PCIECCTLR 0x000018 > -#define CONFIG_SEND_ENABLE BIT(31) > +#define CONFIGURE_SEND_ENABLE BIT(31) The R-Car Gen3 rev. 2.30 Hardware User's Manual calls the bit "CCIE". Hence if I would have written the driver, I would have used #define PCIECCTLR_CCIE BIT(31) /* Configuration Send Enable */ > #define TYPE0 (0 << 8) > #define TYPE1 BIT(8) > #define PCIECDR 0x000020 Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
On Fri, Jan 13, 2023 at 10:05 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > On Fri, Jan 13, 2023 at 9:52 AM Lukas Bulwahn <lukas.bulwahn@gmail.com> wrote: > > Defines prefixed with "CONFIG" should be limited to proper Kconfig options, > > that are introduced in a Kconfig file. > > > > Here, a definition for a bitmask to configure the SEND_ENABLE mode is named > > CONFIG_SEND_ENABLE. > > > > Rename this local definition to CONFIGURE_SEND_ENABLE to avoid defines > > prefixed with "CONFIG". > > > > No functional change. > > > > Signed-off-by: Lukas Bulwahn <lukas.bulwahn@gmail.com> > > Thanks for your patch! > > > --- a/drivers/pci/controller/pcie-rcar.h > > +++ b/drivers/pci/controller/pcie-rcar.h > > @@ -11,7 +11,7 @@ > > > > #define PCIECAR 0x000010 > > #define PCIECCTLR 0x000018 > > -#define CONFIG_SEND_ENABLE BIT(31) > > +#define CONFIGURE_SEND_ENABLE BIT(31) > > The R-Car Gen3 rev. 2.30 Hardware User's Manual calls the bit "CCIE". > > Hence if I would have written the driver, I would have used > > #define PCIECCTLR_CCIE BIT(31) /* Configuration Send Enable */ Regardless: Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Hi Lukas, On Fri, Jan 13, 2023 at 09:45:16AM +0100, Lukas Bulwahn wrote: > Defines prefixed with "CONFIG" should be limited to proper Kconfig options, > that are introduced in a Kconfig file. > > Here, a definition for a bitmask to configure the SEND_ENABLE mode is named > CONFIG_SEND_ENABLE. > > Rename this local definition to CONFIGURE_SEND_ENABLE to avoid defines > prefixed with "CONFIG". > > No functional change. If you update this patch, please capitalize the subject to match: $ git log --oneline drivers/pci/controller/pcie-rcar-host.c 6e36203bc14c ("PCI: rcar: Use PCI_SET_ERROR_RESPONSE after read which triggered an exception") 84b576146294 ("PCI: rcar: Finish transition to L1 state in rcar_pcie_config_access()") d2a14b54989e ("PCI: rcar: Check if device is runtime suspended instead of __clk_is_enabled()") a115b1bd3af0 ("PCI: rcar: Add L1 link state fix into data abort hook") 83ed8d4fa656 ("PCI: rcar: Convert to MSI domains") Bjorn
On Fri, Jan 13, 2023 at 10:05:09AM +0100, Geert Uytterhoeven wrote: > Hi Lukas, > > On Fri, Jan 13, 2023 at 9:52 AM Lukas Bulwahn <lukas.bulwahn@gmail.com> wrote: > > Defines prefixed with "CONFIG" should be limited to proper Kconfig options, > > that are introduced in a Kconfig file. > > > > Here, a definition for a bitmask to configure the SEND_ENABLE mode is named > > CONFIG_SEND_ENABLE. > > > > Rename this local definition to CONFIGURE_SEND_ENABLE to avoid defines > > prefixed with "CONFIG". > > > > No functional change. > > > > Signed-off-by: Lukas Bulwahn <lukas.bulwahn@gmail.com> > > Thanks for your patch! > > > --- a/drivers/pci/controller/pcie-rcar.h > > +++ b/drivers/pci/controller/pcie-rcar.h > > @@ -11,7 +11,7 @@ > > > > #define PCIECAR 0x000010 > > #define PCIECCTLR 0x000018 > > -#define CONFIG_SEND_ENABLE BIT(31) > > +#define CONFIGURE_SEND_ENABLE BIT(31) > > The R-Car Gen3 rev. 2.30 Hardware User's Manual calls the bit "CCIE". > > Hence if I would have written the driver, I would have used > > #define PCIECCTLR_CCIE BIT(31) /* Configuration Send Enable */ Should I change it when I merge it ? That makes sense actually. Thanks, Lorenzo > > #define TYPE0 (0 << 8) > > #define TYPE1 BIT(8) > > #define PCIECDR 0x000020 > > Gr{oetje,eeting}s, > > Geert > > -- > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org > > In personal conversations with technical people, I call myself a hacker. But > when I'm talking to journalists I just say "programmer" or something like that. > -- Linus Torvalds
On Fri, Jan 13, 2023 at 05:30:24PM +0100, Lorenzo Pieralisi wrote: > On Fri, Jan 13, 2023 at 10:05:09AM +0100, Geert Uytterhoeven wrote: > > Hi Lukas, > > > > On Fri, Jan 13, 2023 at 9:52 AM Lukas Bulwahn <lukas.bulwahn@gmail.com> wrote: > > > Defines prefixed with "CONFIG" should be limited to proper Kconfig options, > > > that are introduced in a Kconfig file. > > > > > > Here, a definition for a bitmask to configure the SEND_ENABLE mode is named > > > CONFIG_SEND_ENABLE. > > > > > > Rename this local definition to CONFIGURE_SEND_ENABLE to avoid defines > > > prefixed with "CONFIG". > > > > > > No functional change. > > > > > > Signed-off-by: Lukas Bulwahn <lukas.bulwahn@gmail.com> > > > > Thanks for your patch! > > > > > --- a/drivers/pci/controller/pcie-rcar.h > > > +++ b/drivers/pci/controller/pcie-rcar.h > > > @@ -11,7 +11,7 @@ > > > > > > #define PCIECAR 0x000010 > > > #define PCIECCTLR 0x000018 > > > -#define CONFIG_SEND_ENABLE BIT(31) > > > +#define CONFIGURE_SEND_ENABLE BIT(31) > > > > The R-Car Gen3 rev. 2.30 Hardware User's Manual calls the bit "CCIE". > > > > Hence if I would have written the driver, I would have used > > > > #define PCIECCTLR_CCIE BIT(31) /* Configuration Send Enable */ > > Should I change it when I merge it ? That makes sense actually. If I don't hear from anybody I will make the change above and merge it. Thanks, Lorenzo > > Thanks, > Lorenzo > > > > #define TYPE0 (0 << 8) > > > #define TYPE1 BIT(8) > > > #define PCIECDR 0x000020 > > > > Gr{oetje,eeting}s, > > > > Geert > > > > -- > > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org > > > > In personal conversations with technical people, I call myself a hacker. But > > when I'm talking to journalists I just say "programmer" or something like that. > > -- Linus Torvalds
On Fri, 13 Jan 2023 09:45:16 +0100, Lukas Bulwahn wrote: > Defines prefixed with "CONFIG" should be limited to proper Kconfig options, > that are introduced in a Kconfig file. > > Here, a definition for a bitmask to configure the SEND_ENABLE mode is named > CONFIG_SEND_ENABLE. > > Rename this local definition to CONFIGURE_SEND_ENABLE to avoid defines > prefixed with "CONFIG". > > [...] Applied to pci/rcar, thanks! [1/1] PCI: rcar: avoid defines prefixed with CONFIG https://git.kernel.org/pci/pci/c/727de4c08768 Thanks, Lorenzo
diff --git a/drivers/pci/controller/pcie-rcar-host.c b/drivers/pci/controller/pcie-rcar-host.c index e4faf90feaf5..52fdafe5c877 100644 --- a/drivers/pci/controller/pcie-rcar-host.c +++ b/drivers/pci/controller/pcie-rcar-host.c @@ -219,9 +219,9 @@ static int rcar_pcie_config_access(struct rcar_pcie_host *host, /* Enable the configuration access */ if (pci_is_root_bus(bus->parent)) - rcar_pci_write_reg(pcie, CONFIG_SEND_ENABLE | TYPE0, PCIECCTLR); + rcar_pci_write_reg(pcie, CONFIGURE_SEND_ENABLE | TYPE0, PCIECCTLR); else - rcar_pci_write_reg(pcie, CONFIG_SEND_ENABLE | TYPE1, PCIECCTLR); + rcar_pci_write_reg(pcie, CONFIGURE_SEND_ENABLE | TYPE1, PCIECCTLR); /* Check for errors */ if (rcar_pci_read_reg(pcie, PCIEERRFR) & UNSUPPORTED_REQUEST) diff --git a/drivers/pci/controller/pcie-rcar.h b/drivers/pci/controller/pcie-rcar.h index 9bb125db85c6..a09f58ab04ca 100644 --- a/drivers/pci/controller/pcie-rcar.h +++ b/drivers/pci/controller/pcie-rcar.h @@ -11,7 +11,7 @@ #define PCIECAR 0x000010 #define PCIECCTLR 0x000018 -#define CONFIG_SEND_ENABLE BIT(31) +#define CONFIGURE_SEND_ENABLE BIT(31) #define TYPE0 (0 << 8) #define TYPE1 BIT(8) #define PCIECDR 0x000020
Defines prefixed with "CONFIG" should be limited to proper Kconfig options, that are introduced in a Kconfig file. Here, a definition for a bitmask to configure the SEND_ENABLE mode is named CONFIG_SEND_ENABLE. Rename this local definition to CONFIGURE_SEND_ENABLE to avoid defines prefixed with "CONFIG". No functional change. Signed-off-by: Lukas Bulwahn <lukas.bulwahn@gmail.com> --- drivers/pci/controller/pcie-rcar-host.c | 4 ++-- drivers/pci/controller/pcie-rcar.h | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-)