* Re: [PATCH 2/2] PCI: xilinx-cpm: Add support for Versal CPM5 Root Port controller-1
2024-09-06 9:31 ` [PATCH 2/2] PCI: xilinx-cpm: Add support for Versal CPM5 Root Port controller-1 Thippeswamy Havalige
@ 2024-09-06 9:56 ` Krzysztof Kozlowski
2024-09-06 19:19 ` Bjorn Helgaas
` (3 subsequent siblings)
4 siblings, 0 replies; 15+ messages in thread
From: Krzysztof Kozlowski @ 2024-09-06 9:56 UTC (permalink / raw)
To: Thippeswamy Havalige, manivannan.sadhasivam, robh, linux-pci,
bhelgaas, linux-arm-kernel, linux-kernel, krzk+dt, conor+dt,
devicetree
Cc: bharat.kumar.gogada, michal.simek, lpieralisi, kw
On 06/09/2024 11:31, Thippeswamy Havalige wrote:
> In the CPM5, controller-1 has platform-specific error interrupt bits
> located at different offsets compared to controller-0.
>
> Signed-off-by: Thippeswamy Havalige <thippesw@amd.com>
> ---
> drivers/pci/controller/pcie-xilinx-cpm.c | 39 +++++++++++++++++++-----
> 1 file changed, 32 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/pci/controller/pcie-xilinx-cpm.c b/drivers/pci/controller/pcie-xilinx-cpm.c
> index a0f5e1d67b04..d672f620bc4c 100644
> --- a/drivers/pci/controller/pcie-xilinx-cpm.c
> +++ b/drivers/pci/controller/pcie-xilinx-cpm.c
> @@ -30,10 +30,13 @@
> #define XILINX_CPM_PCIE_REG_IDRN_MASK 0x00000E3C
> #define XILINX_CPM_PCIE_MISC_IR_STATUS 0x00000340
> #define XILINX_CPM_PCIE_MISC_IR_ENABLE 0x00000348
> -#define XILINX_CPM_PCIE_MISC_IR_LOCAL BIT(1)
> +#define XILINX_CPM_PCIE0_MISC_IR_LOCAL BIT(1)
> +#define XILINX_CPM_PCIE1_MISC_IR_LOCAL BIT(2)
>
> -#define XILINX_CPM_PCIE_IR_STATUS 0x000002A0
> -#define XILINX_CPM_PCIE_IR_ENABLE 0x000002A8
> +#define XILINX_CPM_PCIE0_IR_STATUS 0x000002A0
> +#define XILINX_CPM_PCIE1_IR_STATUS 0x000002B4
> +#define XILINX_CPM_PCIE0_IR_ENABLE 0x000002A8
> +#define XILINX_CPM_PCIE1_IR_ENABLE 0x000002BC
> #define XILINX_CPM_PCIE_IR_LOCAL BIT(0)
>
> #define IMR(x) BIT(XILINX_PCIE_INTR_ ##x)
> @@ -280,10 +283,17 @@ static void xilinx_cpm_pcie_event_flow(struct irq_desc *desc)
> pcie_write(port, val, XILINX_CPM_PCIE_REG_IDR);
>
> if (port->variant->version == CPM5) {
> - val = readl_relaxed(port->cpm_base + XILINX_CPM_PCIE_IR_STATUS);
> + val = readl_relaxed(port->cpm_base + XILINX_CPM_PCIE0_IR_STATUS);
> if (val)
> writel_relaxed(val, port->cpm_base +
> - XILINX_CPM_PCIE_IR_STATUS);
> + XILINX_CPM_PCIE0_IR_STATUS);
> + }
> +
There are no blank lines allowed between arms of conditional statements.
Please follow coding style. This case is explained there.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH 2/2] PCI: xilinx-cpm: Add support for Versal CPM5 Root Port controller-1
2024-09-06 9:31 ` [PATCH 2/2] PCI: xilinx-cpm: Add support for Versal CPM5 Root Port controller-1 Thippeswamy Havalige
2024-09-06 9:56 ` Krzysztof Kozlowski
@ 2024-09-06 19:19 ` Bjorn Helgaas
2024-09-12 9:38 ` Havalige, Thippeswamy
2024-09-07 6:15 ` kernel test robot
` (2 subsequent siblings)
4 siblings, 1 reply; 15+ messages in thread
From: Bjorn Helgaas @ 2024-09-06 19:19 UTC (permalink / raw)
To: Thippeswamy Havalige
Cc: manivannan.sadhasivam, robh, linux-pci, bhelgaas,
linux-arm-kernel, linux-kernel, krzk+dt, conor+dt, devicetree,
bharat.kumar.gogada, michal.simek, lpieralisi, kw
On Fri, Sep 06, 2024 at 03:01:48PM +0530, Thippeswamy Havalige wrote:
> In the CPM5, controller-1 has platform-specific error interrupt bits
> located at different offsets compared to controller-0.
Technically mentions a difference between controller-0 and
controller-1, but doesn't say what the patch does.
> Signed-off-by: Thippeswamy Havalige <thippesw@amd.com>
> ---
> drivers/pci/controller/pcie-xilinx-cpm.c | 39 +++++++++++++++++++-----
> 1 file changed, 32 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/pci/controller/pcie-xilinx-cpm.c b/drivers/pci/controller/pcie-xilinx-cpm.c
> index a0f5e1d67b04..d672f620bc4c 100644
> --- a/drivers/pci/controller/pcie-xilinx-cpm.c
> +++ b/drivers/pci/controller/pcie-xilinx-cpm.c
> @@ -30,10 +30,13 @@
> #define XILINX_CPM_PCIE_REG_IDRN_MASK 0x00000E3C
> #define XILINX_CPM_PCIE_MISC_IR_STATUS 0x00000340
> #define XILINX_CPM_PCIE_MISC_IR_ENABLE 0x00000348
> -#define XILINX_CPM_PCIE_MISC_IR_LOCAL BIT(1)
> +#define XILINX_CPM_PCIE0_MISC_IR_LOCAL BIT(1)
> +#define XILINX_CPM_PCIE1_MISC_IR_LOCAL BIT(2)
>
> -#define XILINX_CPM_PCIE_IR_STATUS 0x000002A0
> -#define XILINX_CPM_PCIE_IR_ENABLE 0x000002A8
> +#define XILINX_CPM_PCIE0_IR_STATUS 0x000002A0
> +#define XILINX_CPM_PCIE1_IR_STATUS 0x000002B4
> +#define XILINX_CPM_PCIE0_IR_ENABLE 0x000002A8
> +#define XILINX_CPM_PCIE1_IR_ENABLE 0x000002BC
These are all indented with spaces; should use tabs like the other
definitions above.
> #define XILINX_CPM_PCIE_IR_LOCAL BIT(0)
Same here (although you didn't change this one).
> #define IMR(x) BIT(XILINX_PCIE_INTR_ ##x)
> @@ -280,10 +283,17 @@ static void xilinx_cpm_pcie_event_flow(struct irq_desc *desc)
> pcie_write(port, val, XILINX_CPM_PCIE_REG_IDR);
>
> if (port->variant->version == CPM5) {
> - val = readl_relaxed(port->cpm_base + XILINX_CPM_PCIE_IR_STATUS);
> + val = readl_relaxed(port->cpm_base + XILINX_CPM_PCIE0_IR_STATUS);
> if (val)
> writel_relaxed(val, port->cpm_base +
> - XILINX_CPM_PCIE_IR_STATUS);
> + XILINX_CPM_PCIE0_IR_STATUS);
> + }
> +
> + else if (port->variant->version == CPM5_HOST1) {
> + val = readl_relaxed(port->cpm_base + XILINX_CPM_PCIE1_IR_STATUS);
> + if (val)
> + writel_relaxed(val, port->cpm_base +
> + XILINX_CPM_PCIE1_IR_STATUS);
> }
When possible, I think it would be nicer to encode this difference in
the data, i.e., in struct xilinx_cpm_variant, than in the code. For
example,
struct xilinx_cpm_variant {
enum xilinx_cpm_version version;
+ u32 ir_status;
}
static const struct xilinx_cpm_variant cpm5_host = {
.version = CPM5,
+ .ir_status = XILINX_CPM_PCIE0_IR_STATUS,
};
static const struct xilinx_cpm_variant cpm5_host = {
.version = CPM5_HOST1,
+ .ir_status = XILINX_CPM_PCIE1_IR_STATUS,
};
Then this code could look like this, where it basically tests a
*feature* instead of checking for all the different variants:
struct xilinx_cpm_variant *variant = port->variant;
if (variant->ir_status) {
val = readl_relaxed(port->cpm_base + variant->ir_status);
if (val)
writel_relaxed(port->cpm_base + variant->ir_status);
}
(Apparently the CPM variant doesn't require this at all?)
> /*
> @@ -483,12 +493,19 @@ static void xilinx_cpm_pcie_init_port(struct xilinx_cpm_pcie *port)
> * XILINX_CPM_PCIE_MISC_IR_ENABLE register is mapped to
> * CPM SLCR block.
> */
> - writel(XILINX_CPM_PCIE_MISC_IR_LOCAL,
> + writel(XILINX_CPM_PCIE0_MISC_IR_LOCAL,
> port->cpm_base + XILINX_CPM_PCIE_MISC_IR_ENABLE);
>
> if (port->variant->version == CPM5) {
> writel(XILINX_CPM_PCIE_IR_LOCAL,
> - port->cpm_base + XILINX_CPM_PCIE_IR_ENABLE);
> + port->cpm_base + XILINX_CPM_PCIE0_IR_ENABLE);
> + }
> +
> + else if (port->variant->version == CPM5_HOST1) {
> + writel(XILINX_CPM_PCIE1_MISC_IR_LOCAL,
> + port->cpm_base + XILINX_CPM_PCIE_MISC_IR_ENABLE);
> + writel(XILINX_CPM_PCIE_IR_LOCAL,
> + port->cpm_base + XILINX_CPM_PCIE1_IR_ENABLE);
This looks questionable and worth a comment if this is what you
intend.
CPM
- sets PCIE0_MISC_IR_LOCAL in PCIE_MISC_IR_ENABLE
CPM5
- sets PCIE0_MISC_IR_LOCAL in PCIE_MISC_IR_ENABLE
- sets PCIE_IR_LOCAL in PCIE0_IR_ENABLE
CPM5_HOST1
- sets PCIE0_MISC_IR_LOCAL in PCIE_MISC_IR_ENABLE
- sets PCIE1_MISC_IR_LOCAL in PCIE_MISC_IR_ENABLE (overwrite)
- sets PCIE_IR_LOCAL in PCIE1_IR_ENABLE
The CPM5_HOST1 overwrite looks either wrong or like the first write
was unnecessary.
You might be able to simplify this by adding .misc_ir_value,
.ir_enable, and .ir_local_value:
struct xilinx_cpm_variant *variant = port->variant;
writel(variant->misc_ir_value,
port->cpm_base + XILINX_CPM_PCIE_MISC_IR_ENABLE);
if (variant->ir_enable)
writel(variant->ir_local_value, port->cpm_base + ir_enable);
> /* Enable the Bridge enable bit */
Unrelated to *this* patch except for being in the diff context, but
"enable the enable bit" is unclear because "enable" isn't something
you can do to a bit; you can *set* or *clear* a bit.
Bjorn
^ permalink raw reply [flat|nested] 15+ messages in thread* RE: [PATCH 2/2] PCI: xilinx-cpm: Add support for Versal CPM5 Root Port controller-1
2024-09-06 19:19 ` Bjorn Helgaas
@ 2024-09-12 9:38 ` Havalige, Thippeswamy
0 siblings, 0 replies; 15+ messages in thread
From: Havalige, Thippeswamy @ 2024-09-12 9:38 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: manivannan.sadhasivam@linaro.org, robh@kernel.org,
linux-pci@vger.kernel.org, bhelgaas@google.com,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, krzk+dt@kernel.org,
conor+dt@kernel.org, devicetree@vger.kernel.org,
Gogada, Bharat Kumar, Simek, Michal, lpieralisi@kernel.org,
kw@linux.com
Hi Bjorn,
Thanks, for your comments.
> -----Original Message-----
> From: Bjorn Helgaas <helgaas@kernel.org>
> Sent: Saturday, September 7, 2024 12:49 AM
> To: Havalige, Thippeswamy <thippeswamy.havalige@amd.com>
> Cc: manivannan.sadhasivam@linaro.org; robh@kernel.org; linux-
> pci@vger.kernel.org; bhelgaas@google.com; linux-arm-kernel@lists.infradead.org;
> linux-kernel@vger.kernel.org; krzk+dt@kernel.org; conor+dt@kernel.org;
> devicetree@vger.kernel.org; Gogada, Bharat Kumar
> <bharat.kumar.gogada@amd.com>; Simek, Michal <michal.simek@amd.com>;
> lpieralisi@kernel.org; kw@linux.com
> Subject: Re: [PATCH 2/2] PCI: xilinx-cpm: Add support for Versal CPM5 Root Port
> controller-1
>
> On Fri, Sep 06, 2024 at 03:01:48PM +0530, Thippeswamy Havalige wrote:
> > In the CPM5, controller-1 has platform-specific error interrupt bits
> > located at different offsets compared to controller-0.
>
> Technically mentions a difference between controller-0 and
> controller-1, but doesn't say what the patch does.
I'll update required detailed description in next patch.
>
> > Signed-off-by: Thippeswamy Havalige <thippesw@amd.com>
> > ---
> > drivers/pci/controller/pcie-xilinx-cpm.c | 39 +++++++++++++++++++-----
> > 1 file changed, 32 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/pci/controller/pcie-xilinx-cpm.c b/drivers/pci/controller/pcie-
> xilinx-cpm.c
> > index a0f5e1d67b04..d672f620bc4c 100644
> > --- a/drivers/pci/controller/pcie-xilinx-cpm.c
> > +++ b/drivers/pci/controller/pcie-xilinx-cpm.c
> > @@ -30,10 +30,13 @@
> > #define XILINX_CPM_PCIE_REG_IDRN_MASK 0x00000E3C
> > #define XILINX_CPM_PCIE_MISC_IR_STATUS 0x00000340
> > #define XILINX_CPM_PCIE_MISC_IR_ENABLE 0x00000348
> > -#define XILINX_CPM_PCIE_MISC_IR_LOCAL BIT(1)
> > +#define XILINX_CPM_PCIE0_MISC_IR_LOCAL BIT(1)
> > +#define XILINX_CPM_PCIE1_MISC_IR_LOCAL BIT(2)
> >
> > -#define XILINX_CPM_PCIE_IR_STATUS 0x000002A0
> > -#define XILINX_CPM_PCIE_IR_ENABLE 0x000002A8
> > +#define XILINX_CPM_PCIE0_IR_STATUS 0x000002A0
> > +#define XILINX_CPM_PCIE1_IR_STATUS 0x000002B4
> > +#define XILINX_CPM_PCIE0_IR_ENABLE 0x000002A8
> > +#define XILINX_CPM_PCIE1_IR_ENABLE 0x000002BC
>
> These are all indented with spaces; should use tabs like the other
> definitions above.
Thanks, will add proper tabs in next patch.
>
> > #define XILINX_CPM_PCIE_IR_LOCAL BIT(0)
>
> Same here (although you didn't change this one).
>
> > #define IMR(x) BIT(XILINX_PCIE_INTR_ ##x)
> > @@ -280,10 +283,17 @@ static void xilinx_cpm_pcie_event_flow(struct
> irq_desc *desc)
> > pcie_write(port, val, XILINX_CPM_PCIE_REG_IDR);
> >
> > if (port->variant->version == CPM5) {
> > - val = readl_relaxed(port->cpm_base +
> XILINX_CPM_PCIE_IR_STATUS);
> > + val = readl_relaxed(port->cpm_base +
> XILINX_CPM_PCIE0_IR_STATUS);
> > if (val)
> > writel_relaxed(val, port->cpm_base +
> > - XILINX_CPM_PCIE_IR_STATUS);
> > + XILINX_CPM_PCIE0_IR_STATUS);
> > + }
> > +
> > + else if (port->variant->version == CPM5_HOST1) {
> > + val = readl_relaxed(port->cpm_base +
> XILINX_CPM_PCIE1_IR_STATUS);
> > + if (val)
> > + writel_relaxed(val, port->cpm_base +
> > + XILINX_CPM_PCIE1_IR_STATUS);
> > }
>
> When possible, I think it would be nicer to encode this difference in
> the data, i.e., in struct xilinx_cpm_variant, than in the code. For
> example,
>
> struct xilinx_cpm_variant {
> enum xilinx_cpm_version version;
> + u32 ir_status;
> }
>
> static const struct xilinx_cpm_variant cpm5_host = {
> .version = CPM5,
> + .ir_status = XILINX_CPM_PCIE0_IR_STATUS,
> };
>
> static const struct xilinx_cpm_variant cpm5_host = {
> .version = CPM5_HOST1,
> + .ir_status = XILINX_CPM_PCIE1_IR_STATUS,
> };
>
> Then this code could look like this, where it basically tests a
> *feature* instead of checking for all the different variants:
>
> struct xilinx_cpm_variant *variant = port->variant;
>
> if (variant->ir_status) {
> val = readl_relaxed(port->cpm_base + variant->ir_status);
> if (val)
> writel_relaxed(port->cpm_base + variant->ir_status);
> }
>
> (Apparently the CPM variant doesn't require this at all?)
>
> > /*
> > @@ -483,12 +493,19 @@ static void xilinx_cpm_pcie_init_port(struct
> xilinx_cpm_pcie *port)
> > * XILINX_CPM_PCIE_MISC_IR_ENABLE register is mapped to
> > * CPM SLCR block.
> > */
> > - writel(XILINX_CPM_PCIE_MISC_IR_LOCAL,
> > + writel(XILINX_CPM_PCIE0_MISC_IR_LOCAL,
> > port->cpm_base + XILINX_CPM_PCIE_MISC_IR_ENABLE);
> >
> > if (port->variant->version == CPM5) {
> > writel(XILINX_CPM_PCIE_IR_LOCAL,
> > - port->cpm_base + XILINX_CPM_PCIE_IR_ENABLE);
> > + port->cpm_base + XILINX_CPM_PCIE0_IR_ENABLE);
> > + }
> > +
> > + else if (port->variant->version == CPM5_HOST1) {
> > + writel(XILINX_CPM_PCIE1_MISC_IR_LOCAL,
> > + port->cpm_base + XILINX_CPM_PCIE_MISC_IR_ENABLE);
> > + writel(XILINX_CPM_PCIE_IR_LOCAL,
> > + port->cpm_base + XILINX_CPM_PCIE1_IR_ENABLE);
>
> This looks questionable and worth a comment if this is what you
> intend.
>
> CPM
> - sets PCIE0_MISC_IR_LOCAL in PCIE_MISC_IR_ENABLE
>
> CPM5
> - sets PCIE0_MISC_IR_LOCAL in PCIE_MISC_IR_ENABLE
> - sets PCIE_IR_LOCAL in PCIE0_IR_ENABLE
>
> CPM5_HOST1
> - sets PCIE0_MISC_IR_LOCAL in PCIE_MISC_IR_ENABLE
> - sets PCIE1_MISC_IR_LOCAL in PCIE_MISC_IR_ENABLE (overwrite)
> - sets PCIE_IR_LOCAL in PCIE1_IR_ENABLE
>
> The CPM5_HOST1 overwrite looks either wrong or like the first write
> was unnecessary.
Yes, the initial write is unnecessary here. I will fix this in next patch.
>
> You might be able to simplify this by adding .misc_ir_value,
> .ir_enable, and .ir_local_value:
>
> struct xilinx_cpm_variant *variant = port->variant;
>
> writel(variant->misc_ir_value,
> port->cpm_base + XILINX_CPM_PCIE_MISC_IR_ENABLE);
> if (variant->ir_enable)
> writel(variant->ir_local_value, port->cpm_base + ir_enable);
>
> > /* Enable the Bridge enable bit */
>
> Unrelated to *this* patch except for being in the diff context, but
> "enable the enable bit" is unclear because "enable" isn't something
> you can do to a bit; you can *set* or *clear* a bit.
>
> Bjorn
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/2] PCI: xilinx-cpm: Add support for Versal CPM5 Root Port controller-1
2024-09-06 9:31 ` [PATCH 2/2] PCI: xilinx-cpm: Add support for Versal CPM5 Root Port controller-1 Thippeswamy Havalige
2024-09-06 9:56 ` Krzysztof Kozlowski
2024-09-06 19:19 ` Bjorn Helgaas
@ 2024-09-07 6:15 ` kernel test robot
2024-09-07 9:00 ` kernel test robot
2024-09-07 10:01 ` kernel test robot
4 siblings, 0 replies; 15+ messages in thread
From: kernel test robot @ 2024-09-07 6:15 UTC (permalink / raw)
To: Thippeswamy Havalige, manivannan.sadhasivam, robh, linux-pci,
bhelgaas, linux-arm-kernel, linux-kernel, krzk+dt, conor+dt,
devicetree
Cc: oe-kbuild-all, bharat.kumar.gogada, michal.simek, lpieralisi, kw,
Thippeswamy Havalige
Hi Thippeswamy,
kernel test robot noticed the following build errors:
[auto build test ERROR on pci/next]
[also build test ERROR on pci/for-linus mani-mhi/mhi-next robh/for-next linus/master v6.11-rc6 next-20240906]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Thippeswamy-Havalige/dt-bindings-PCI-xilinx-cpm-Add-compatible-string-for-CPM5-controller-1/20240906-173446
base: https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git next
patch link: https://lore.kernel.org/r/20240906093148.830452-3-thippesw%40amd.com
patch subject: [PATCH 2/2] PCI: xilinx-cpm: Add support for Versal CPM5 Root Port controller-1
config: alpha-randconfig-r051-20240907 (https://download.01.org/0day-ci/archive/20240907/202409071415.6WivnBm0-lkp@intel.com/config)
compiler: alpha-linux-gcc (GCC) 13.3.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240907/202409071415.6WivnBm0-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202409071415.6WivnBm0-lkp@intel.com/
All errors (new ones prefixed by >>):
drivers/pci/controller/pcie-xilinx-cpm.c: In function 'xilinx_cpm_pcie_event_flow':
>> drivers/pci/controller/pcie-xilinx-cpm.c:292:44: error: 'CPM5_HOST1' undeclared (first use in this function)
292 | else if (port->variant->version == CPM5_HOST1) {
| ^~~~~~~~~~
drivers/pci/controller/pcie-xilinx-cpm.c:292:44: note: each undeclared identifier is reported only once for each function it appears in
drivers/pci/controller/pcie-xilinx-cpm.c: In function 'xilinx_cpm_pcie_init_port':
drivers/pci/controller/pcie-xilinx-cpm.c:504:44: error: 'CPM5_HOST1' undeclared (first use in this function)
504 | else if (port->variant->version == CPM5_HOST1) {
| ^~~~~~~~~~
drivers/pci/controller/pcie-xilinx-cpm.c: At top level:
>> drivers/pci/controller/pcie-xilinx-cpm.c:635:40: error: redefinition of 'cpm5_host'
635 | static const struct xilinx_cpm_variant cpm5_host = {
| ^~~~~~~~~
drivers/pci/controller/pcie-xilinx-cpm.c:631:40: note: previous definition of 'cpm5_host' with type 'const struct xilinx_cpm_variant'
631 | static const struct xilinx_cpm_variant cpm5_host = {
| ^~~~~~~~~
>> drivers/pci/controller/pcie-xilinx-cpm.c:636:20: error: 'CPM5_HOST1' undeclared here (not in a function)
636 | .version = CPM5_HOST1,
| ^~~~~~~~~~
>> drivers/pci/controller/pcie-xilinx-cpm.c:650:26: error: 'cpm5_host1' undeclared here (not in a function); did you mean 'cpm5_host'?
650 | .data = &cpm5_host1,
| ^~~~~~~~~~
| cpm5_host
vim +/CPM5_HOST1 +292 drivers/pci/controller/pcie-xilinx-cpm.c
270
271 static void xilinx_cpm_pcie_event_flow(struct irq_desc *desc)
272 {
273 struct xilinx_cpm_pcie *port = irq_desc_get_handler_data(desc);
274 struct irq_chip *chip = irq_desc_get_chip(desc);
275 unsigned long val;
276 int i;
277
278 chained_irq_enter(chip, desc);
279 val = pcie_read(port, XILINX_CPM_PCIE_REG_IDR);
280 val &= pcie_read(port, XILINX_CPM_PCIE_REG_IMR);
281 for_each_set_bit(i, &val, 32)
282 generic_handle_domain_irq(port->cpm_domain, i);
283 pcie_write(port, val, XILINX_CPM_PCIE_REG_IDR);
284
285 if (port->variant->version == CPM5) {
286 val = readl_relaxed(port->cpm_base + XILINX_CPM_PCIE0_IR_STATUS);
287 if (val)
288 writel_relaxed(val, port->cpm_base +
289 XILINX_CPM_PCIE0_IR_STATUS);
290 }
291
> 292 else if (port->variant->version == CPM5_HOST1) {
293 val = readl_relaxed(port->cpm_base + XILINX_CPM_PCIE1_IR_STATUS);
294 if (val)
295 writel_relaxed(val, port->cpm_base +
296 XILINX_CPM_PCIE1_IR_STATUS);
297 }
298
299 /*
300 * XILINX_CPM_PCIE_MISC_IR_STATUS register is mapped to
301 * CPM SLCR block.
302 */
303 val = readl_relaxed(port->cpm_base + XILINX_CPM_PCIE_MISC_IR_STATUS);
304 if (val)
305 writel_relaxed(val,
306 port->cpm_base + XILINX_CPM_PCIE_MISC_IR_STATUS);
307
308 chained_irq_exit(chip, desc);
309 }
310
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH 2/2] PCI: xilinx-cpm: Add support for Versal CPM5 Root Port controller-1
2024-09-06 9:31 ` [PATCH 2/2] PCI: xilinx-cpm: Add support for Versal CPM5 Root Port controller-1 Thippeswamy Havalige
` (2 preceding siblings ...)
2024-09-07 6:15 ` kernel test robot
@ 2024-09-07 9:00 ` kernel test robot
2024-09-07 10:01 ` kernel test robot
4 siblings, 0 replies; 15+ messages in thread
From: kernel test robot @ 2024-09-07 9:00 UTC (permalink / raw)
To: Thippeswamy Havalige, manivannan.sadhasivam, robh, linux-pci,
bhelgaas, linux-arm-kernel, linux-kernel, krzk+dt, conor+dt,
devicetree
Cc: llvm, oe-kbuild-all, bharat.kumar.gogada, michal.simek,
lpieralisi, kw, Thippeswamy Havalige
Hi Thippeswamy,
kernel test robot noticed the following build errors:
[auto build test ERROR on pci/next]
[also build test ERROR on pci/for-linus mani-mhi/mhi-next robh/for-next linus/master v6.11-rc6 next-20240906]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Thippeswamy-Havalige/dt-bindings-PCI-xilinx-cpm-Add-compatible-string-for-CPM5-controller-1/20240906-173446
base: https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git next
patch link: https://lore.kernel.org/r/20240906093148.830452-3-thippesw%40amd.com
patch subject: [PATCH 2/2] PCI: xilinx-cpm: Add support for Versal CPM5 Root Port controller-1
config: s390-allmodconfig (https://download.01.org/0day-ci/archive/20240907/202409071635.vNwvSZtg-lkp@intel.com/config)
compiler: clang version 20.0.0git (https://github.com/llvm/llvm-project 05f5a91d00b02f4369f46d076411c700755ae041)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240907/202409071635.vNwvSZtg-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202409071635.vNwvSZtg-lkp@intel.com/
All errors (new ones prefixed by >>):
In file included from drivers/pci/controller/pcie-xilinx-cpm.c:10:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:14:
In file included from arch/s390/include/asm/io.h:93:
include/asm-generic/io.h:548:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
548 | val = __raw_readb(PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:561:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
561 | val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
| ~~~~~~~~~~ ^
include/uapi/linux/byteorder/big_endian.h:37:59: note: expanded from macro '__le16_to_cpu'
37 | #define __le16_to_cpu(x) __swab16((__force __u16)(__le16)(x))
| ^
include/uapi/linux/swab.h:102:54: note: expanded from macro '__swab16'
102 | #define __swab16(x) (__u16)__builtin_bswap16((__u16)(x))
| ^
In file included from drivers/pci/controller/pcie-xilinx-cpm.c:10:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:14:
In file included from arch/s390/include/asm/io.h:93:
include/asm-generic/io.h:574:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
574 | val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
| ~~~~~~~~~~ ^
include/uapi/linux/byteorder/big_endian.h:35:59: note: expanded from macro '__le32_to_cpu'
35 | #define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x))
| ^
include/uapi/linux/swab.h:115:54: note: expanded from macro '__swab32'
115 | #define __swab32(x) (__u32)__builtin_bswap32((__u32)(x))
| ^
In file included from drivers/pci/controller/pcie-xilinx-cpm.c:10:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:14:
In file included from arch/s390/include/asm/io.h:93:
include/asm-generic/io.h:585:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
585 | __raw_writeb(value, PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:595:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
595 | __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:605:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
605 | __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:693:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
693 | readsb(PCI_IOBASE + addr, buffer, count);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:701:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
701 | readsw(PCI_IOBASE + addr, buffer, count);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:709:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
709 | readsl(PCI_IOBASE + addr, buffer, count);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:718:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
718 | writesb(PCI_IOBASE + addr, buffer, count);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:727:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
727 | writesw(PCI_IOBASE + addr, buffer, count);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:736:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
736 | writesl(PCI_IOBASE + addr, buffer, count);
| ~~~~~~~~~~ ^
In file included from drivers/pci/controller/pcie-xilinx-cpm.c:10:
In file included from include/linux/irq.h:591:
In file included from arch/s390/include/asm/hw_irq.h:6:
In file included from include/linux/pci.h:37:
In file included from include/linux/device.h:32:
In file included from include/linux/device/driver.h:21:
In file included from include/linux/module.h:19:
In file included from include/linux/elf.h:6:
In file included from arch/s390/include/asm/elf.h:181:
In file included from arch/s390/include/asm/mmu_context.h:11:
In file included from arch/s390/include/asm/pgalloc.h:18:
In file included from include/linux/mm.h:2228:
include/linux/vmstat.h:500:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
500 | return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
| ~~~~~~~~~~~~~~~~~~~~~ ^
501 | item];
| ~~~~
include/linux/vmstat.h:507:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
507 | return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
| ~~~~~~~~~~~~~~~~~~~~~ ^
508 | NR_VM_NUMA_EVENT_ITEMS +
| ~~~~~~~~~~~~~~~~~~~~~~
include/linux/vmstat.h:514:36: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
514 | return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
| ~~~~~~~~~~~ ^ ~~~
include/linux/vmstat.h:519:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
519 | return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
| ~~~~~~~~~~~~~~~~~~~~~ ^
520 | NR_VM_NUMA_EVENT_ITEMS +
| ~~~~~~~~~~~~~~~~~~~~~~
include/linux/vmstat.h:528:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
528 | return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
| ~~~~~~~~~~~~~~~~~~~~~ ^
529 | NR_VM_NUMA_EVENT_ITEMS +
| ~~~~~~~~~~~~~~~~~~~~~~
>> drivers/pci/controller/pcie-xilinx-cpm.c:292:37: error: use of undeclared identifier 'CPM5_HOST1'
292 | else if (port->variant->version == CPM5_HOST1) {
| ^
drivers/pci/controller/pcie-xilinx-cpm.c:504:37: error: use of undeclared identifier 'CPM5_HOST1'
504 | else if (port->variant->version == CPM5_HOST1) {
| ^
drivers/pci/controller/pcie-xilinx-cpm.c:636:13: error: use of undeclared identifier 'CPM5_HOST1'
636 | .version = CPM5_HOST1,
| ^
>> drivers/pci/controller/pcie-xilinx-cpm.c:650:12: error: use of undeclared identifier 'cpm5_host1'; did you mean 'cpm5_host'?
650 | .data = &cpm5_host1,
| ^~~~~~~~~~
| cpm5_host
drivers/pci/controller/pcie-xilinx-cpm.c:635:40: note: 'cpm5_host' declared here
635 | static const struct xilinx_cpm_variant cpm5_host = {
| ^
17 warnings and 4 errors generated.
vim +/CPM5_HOST1 +292 drivers/pci/controller/pcie-xilinx-cpm.c
270
271 static void xilinx_cpm_pcie_event_flow(struct irq_desc *desc)
272 {
273 struct xilinx_cpm_pcie *port = irq_desc_get_handler_data(desc);
274 struct irq_chip *chip = irq_desc_get_chip(desc);
275 unsigned long val;
276 int i;
277
278 chained_irq_enter(chip, desc);
279 val = pcie_read(port, XILINX_CPM_PCIE_REG_IDR);
280 val &= pcie_read(port, XILINX_CPM_PCIE_REG_IMR);
281 for_each_set_bit(i, &val, 32)
282 generic_handle_domain_irq(port->cpm_domain, i);
283 pcie_write(port, val, XILINX_CPM_PCIE_REG_IDR);
284
285 if (port->variant->version == CPM5) {
286 val = readl_relaxed(port->cpm_base + XILINX_CPM_PCIE0_IR_STATUS);
287 if (val)
288 writel_relaxed(val, port->cpm_base +
289 XILINX_CPM_PCIE0_IR_STATUS);
290 }
291
> 292 else if (port->variant->version == CPM5_HOST1) {
293 val = readl_relaxed(port->cpm_base + XILINX_CPM_PCIE1_IR_STATUS);
294 if (val)
295 writel_relaxed(val, port->cpm_base +
296 XILINX_CPM_PCIE1_IR_STATUS);
297 }
298
299 /*
300 * XILINX_CPM_PCIE_MISC_IR_STATUS register is mapped to
301 * CPM SLCR block.
302 */
303 val = readl_relaxed(port->cpm_base + XILINX_CPM_PCIE_MISC_IR_STATUS);
304 if (val)
305 writel_relaxed(val,
306 port->cpm_base + XILINX_CPM_PCIE_MISC_IR_STATUS);
307
308 chained_irq_exit(chip, desc);
309 }
310
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH 2/2] PCI: xilinx-cpm: Add support for Versal CPM5 Root Port controller-1
2024-09-06 9:31 ` [PATCH 2/2] PCI: xilinx-cpm: Add support for Versal CPM5 Root Port controller-1 Thippeswamy Havalige
` (3 preceding siblings ...)
2024-09-07 9:00 ` kernel test robot
@ 2024-09-07 10:01 ` kernel test robot
4 siblings, 0 replies; 15+ messages in thread
From: kernel test robot @ 2024-09-07 10:01 UTC (permalink / raw)
To: Thippeswamy Havalige, manivannan.sadhasivam, robh, linux-pci,
bhelgaas, linux-arm-kernel, linux-kernel, krzk+dt, conor+dt,
devicetree
Cc: oe-kbuild-all, bharat.kumar.gogada, michal.simek, lpieralisi, kw,
Thippeswamy Havalige
Hi Thippeswamy,
kernel test robot noticed the following build warnings:
[auto build test WARNING on pci/next]
[also build test WARNING on pci/for-linus mani-mhi/mhi-next robh/for-next linus/master v6.11-rc6 next-20240906]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Thippeswamy-Havalige/dt-bindings-PCI-xilinx-cpm-Add-compatible-string-for-CPM5-controller-1/20240906-173446
base: https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git next
patch link: https://lore.kernel.org/r/20240906093148.830452-3-thippesw%40amd.com
patch subject: [PATCH 2/2] PCI: xilinx-cpm: Add support for Versal CPM5 Root Port controller-1
config: um-allyesconfig (https://download.01.org/0day-ci/archive/20240907/202409071713.wrAI0UuK-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240907/202409071713.wrAI0UuK-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202409071713.wrAI0UuK-lkp@intel.com/
All warnings (new ones prefixed by >>):
drivers/pci/controller/pcie-xilinx-cpm.c: In function 'xilinx_cpm_pcie_event_flow':
drivers/pci/controller/pcie-xilinx-cpm.c:292:44: error: 'CPM5_HOST1' undeclared (first use in this function)
292 | else if (port->variant->version == CPM5_HOST1) {
| ^~~~~~~~~~
drivers/pci/controller/pcie-xilinx-cpm.c:292:44: note: each undeclared identifier is reported only once for each function it appears in
drivers/pci/controller/pcie-xilinx-cpm.c: In function 'xilinx_cpm_pcie_init_port':
drivers/pci/controller/pcie-xilinx-cpm.c:504:44: error: 'CPM5_HOST1' undeclared (first use in this function)
504 | else if (port->variant->version == CPM5_HOST1) {
| ^~~~~~~~~~
drivers/pci/controller/pcie-xilinx-cpm.c: At top level:
drivers/pci/controller/pcie-xilinx-cpm.c:635:40: error: redefinition of 'cpm5_host'
635 | static const struct xilinx_cpm_variant cpm5_host = {
| ^~~~~~~~~
drivers/pci/controller/pcie-xilinx-cpm.c:631:40: note: previous definition of 'cpm5_host' with type 'const struct xilinx_cpm_variant'
631 | static const struct xilinx_cpm_variant cpm5_host = {
| ^~~~~~~~~
drivers/pci/controller/pcie-xilinx-cpm.c:636:20: error: 'CPM5_HOST1' undeclared here (not in a function)
636 | .version = CPM5_HOST1,
| ^~~~~~~~~~
drivers/pci/controller/pcie-xilinx-cpm.c:650:26: error: 'cpm5_host1' undeclared here (not in a function); did you mean 'cpm5_host'?
650 | .data = &cpm5_host1,
| ^~~~~~~~~~
| cpm5_host
>> drivers/pci/controller/pcie-xilinx-cpm.c:631:40: warning: 'cpm5_host' defined but not used [-Wunused-const-variable=]
631 | static const struct xilinx_cpm_variant cpm5_host = {
| ^~~~~~~~~
vim +/cpm5_host +631 drivers/pci/controller/pcie-xilinx-cpm.c
51f1ffc00d95e3 Bharat Kumar Gogada 2022-07-05 630
51f1ffc00d95e3 Bharat Kumar Gogada 2022-07-05 @631 static const struct xilinx_cpm_variant cpm5_host = {
51f1ffc00d95e3 Bharat Kumar Gogada 2022-07-05 632 .version = CPM5,
51f1ffc00d95e3 Bharat Kumar Gogada 2022-07-05 633 };
51f1ffc00d95e3 Bharat Kumar Gogada 2022-07-05 634
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 15+ messages in thread