[PATCH 0/3] phy: ralink: phy-mt7621-pci: some fixes for COMPILE_TEST

[PATCH 0/3] phy: ralink: phy-mt7621-pci: some fixes for COMPILE_TEST

[Sergio Paracuellos]

Hi Vinod,

After merging the phy-next tree, today's linux-next build (x86_64
allmodconfig) failed like this:

drivers/phy/ralink/phy-mt7621-pci.c:17:10: fatal error: mt7621.h: No such file or directory
17 | #include <mt7621.h>
 |          ^~~~~~~~~~

This driver has two includes which are in
"arch/mips/include/asm/mach-ralink" and are directly included in the
driver:
* mt7621.h
* ralink_regs.h

This is because this path is directly included in 'arch/mips/ralink/Platform'
for "ralink".

Adding the include search path to the phy ralink directory fix the problem.

Making 'allmodconfig' in x86_64 also showed two warnings for this driver
because of pointer size and not valid name for MODULE_DEVICE_TABLE ids.

This two have been also fixed and included in this series.

Best regards,
    Sergio Paracuellos

Sergio Paracuellos (3):
  phy: ralink: phy-mt7621-pci: add include search path in Makefile
  phy: ralink: phy-mt7621-pci: avoid warning if COMPILE_TEST is enabled
  phy: ralink: phy-mt7621-pci: set correct name in MODULE_DEVICE_TABLE
    macro

 drivers/phy/ralink/Makefile         | 3 +++
 drivers/phy/ralink/phy-mt7621-pci.c | 6 +++---
 2 files changed, 6 insertions(+), 3 deletions(-)

-- 
2.25.1

[PATCH 1/3] phy: ralink: phy-mt7621-pci: add include search path in Makefile

[Sergio Paracuellos]

This driver includes the following two files directly:
- mt7621.h
- ralink_regs.h

Compilation for its related platform properly works because
its real path is included in 'arch/mips/ralink/Platform' as
cflags.

This driver depends on RALINK but also is enabled for COMPILE_TEST
where nothing about its platform is known and this directly
included files are not found at all breaking compilation.

Fix this problem adding include search path for ralink in
ralink phy directory Makefile.

Fixes: d87da32372a0 ("phy: ralink: Add PHY driver for MT7621 PCIe PHY")
Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 drivers/phy/ralink/Makefile | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/phy/ralink/Makefile b/drivers/phy/ralink/Makefile
index cda2a4a7ca5e..c8f9adba0d82 100644
--- a/drivers/phy/ralink/Makefile
+++ b/drivers/phy/ralink/Makefile
@@ -1,3 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0-only
+
+ccflags-y	+= -I$(srctree)/arch/mips/include/asm/mach-ralink
+
 obj-$(CONFIG_PHY_MT7621_PCI)	+= phy-mt7621-pci.o
 obj-$(CONFIG_PHY_RALINK_USB)	+= phy-ralink-usb.o
-- 
2.25.1

Re: [PATCH 1/3] phy: ralink: phy-mt7621-pci: add include search path in Makefile

[Vinod Koul]

On 01-12-20, 11:16, Sergio Paracuellos wrote:
> This driver includes the following two files directly:
> - mt7621.h
> - ralink_regs.h
> 
> Compilation for its related platform properly works because
> its real path is included in 'arch/mips/ralink/Platform' as
> cflags.
> 
> This driver depends on RALINK but also is enabled for COMPILE_TEST
> where nothing about its platform is known and this directly
> included files are not found at all breaking compilation.
> 
> Fix this problem adding include search path for ralink in
> ralink phy directory Makefile.
> 
> Fixes: d87da32372a0 ("phy: ralink: Add PHY driver for MT7621 PCIe PHY")

Pls add reported-by sfr..

> Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
> ---
>  drivers/phy/ralink/Makefile | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/phy/ralink/Makefile b/drivers/phy/ralink/Makefile
> index cda2a4a7ca5e..c8f9adba0d82 100644
> --- a/drivers/phy/ralink/Makefile
> +++ b/drivers/phy/ralink/Makefile
> @@ -1,3 +1,6 @@
>  # SPDX-License-Identifier: GPL-2.0-only
> +
> +ccflags-y	+= -I$(srctree)/arch/mips/include/asm/mach-ralink

Can we include <asm/mips/...> instead of using this?

> +
>  obj-$(CONFIG_PHY_MT7621_PCI)	+= phy-mt7621-pci.o
>  obj-$(CONFIG_PHY_RALINK_USB)	+= phy-ralink-usb.o
> -- 
> 2.25.1

-- 
~Vinod

Re: [PATCH 1/3] phy: ralink: phy-mt7621-pci: add include search path in Makefile

[Sergio Paracuellos]

Hi Vinod,

On Tue, Dec 1, 2020 at 11:42 AM Vinod Koul <vkoul@kernel.org> wrote:
>
> On 01-12-20, 11:16, Sergio Paracuellos wrote:
> > This driver includes the following two files directly:
> > - mt7621.h
> > - ralink_regs.h
> >
> > Compilation for its related platform properly works because
> > its real path is included in 'arch/mips/ralink/Platform' as
> > cflags.
> >
> > This driver depends on RALINK but also is enabled for COMPILE_TEST
> > where nothing about its platform is known and this directly
> > included files are not found at all breaking compilation.
> >
> > Fix this problem adding include search path for ralink in
> > ralink phy directory Makefile.
> >
> > Fixes: d87da32372a0 ("phy: ralink: Add PHY driver for MT7621 PCIe PHY")
>
> Pls add reported-by sfr..

Ok, Will change this and send v2.

>
> > Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
> > ---
> >  drivers/phy/ralink/Makefile | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/drivers/phy/ralink/Makefile b/drivers/phy/ralink/Makefile
> > index cda2a4a7ca5e..c8f9adba0d82 100644
> > --- a/drivers/phy/ralink/Makefile
> > +++ b/drivers/phy/ralink/Makefile
> > @@ -1,3 +1,6 @@
> >  # SPDX-License-Identifier: GPL-2.0-only
> > +
> > +ccflags-y    += -I$(srctree)/arch/mips/include/asm/mach-ralink
>
> Can we include <asm/mips/...> instead of using this?

Doing that also fails because path for 'arch/mips/include' is not
added also because we are compiling in x86_64.
Looking into other drivers I see the way to avoid this kind of
platform specific stuff is adding related paths in
cflags. So I don't really know if there is another way.

>
> > +
> >  obj-$(CONFIG_PHY_MT7621_PCI) += phy-mt7621-pci.o
> >  obj-$(CONFIG_PHY_RALINK_USB) += phy-ralink-usb.o
> > --
> > 2.25.1
>
> --
> ~Vinod

Best regards,
    Sergio Paracuellos

Re: [PATCH 1/3] phy: ralink: phy-mt7621-pci: add include search path in Makefile

[Vinod Koul]

On 01-12-20, 11:47, Sergio Paracuellos wrote:
> Hi Vinod,
> 
> On Tue, Dec 1, 2020 at 11:42 AM Vinod Koul <vkoul@kernel.org> wrote:
> >
> > On 01-12-20, 11:16, Sergio Paracuellos wrote:
> > > This driver includes the following two files directly:
> > > - mt7621.h
> > > - ralink_regs.h
> > >
> > > Compilation for its related platform properly works because
> > > its real path is included in 'arch/mips/ralink/Platform' as
> > > cflags.
> > >
> > > This driver depends on RALINK but also is enabled for COMPILE_TEST
> > > where nothing about its platform is known and this directly
> > > included files are not found at all breaking compilation.
> > >
> > > Fix this problem adding include search path for ralink in
> > > ralink phy directory Makefile.
> > >
> > > Fixes: d87da32372a0 ("phy: ralink: Add PHY driver for MT7621 PCIe PHY")
> >
> > Pls add reported-by sfr..
> 
> Ok, Will change this and send v2.
> 
> >
> > > Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
> > > ---
> > >  drivers/phy/ralink/Makefile | 3 +++
> > >  1 file changed, 3 insertions(+)
> > >
> > > diff --git a/drivers/phy/ralink/Makefile b/drivers/phy/ralink/Makefile
> > > index cda2a4a7ca5e..c8f9adba0d82 100644
> > > --- a/drivers/phy/ralink/Makefile
> > > +++ b/drivers/phy/ralink/Makefile
> > > @@ -1,3 +1,6 @@
> > >  # SPDX-License-Identifier: GPL-2.0-only
> > > +
> > > +ccflags-y    += -I$(srctree)/arch/mips/include/asm/mach-ralink
> >
> > Can we include <asm/mips/...> instead of using this?
> 
> Doing that also fails because path for 'arch/mips/include' is not
> added also because we are compiling in x86_64.
> Looking into other drivers I see the way to avoid this kind of
> platform specific stuff is adding related paths in
> cflags. So I don't really know if there is another way.

Right, ideally these headers should have been in include/linux, but.. I
dont like this way, can we drop compile test..?

-- 
~Vinod

Re: [PATCH 1/3] phy: ralink: phy-mt7621-pci: add include search path in Makefile

[Sergio Paracuellos]

Hi Vinod,

On Tue, Dec 1, 2020 at 12:02 PM Vinod Koul <vkoul@kernel.org> wrote:
>
> On 01-12-20, 11:47, Sergio Paracuellos wrote:
> > Hi Vinod,
> >
> > On Tue, Dec 1, 2020 at 11:42 AM Vinod Koul <vkoul@kernel.org> wrote:
> > >
> > > On 01-12-20, 11:16, Sergio Paracuellos wrote:
> > > > This driver includes the following two files directly:
> > > > - mt7621.h
> > > > - ralink_regs.h
> > > >
> > > > Compilation for its related platform properly works because
> > > > its real path is included in 'arch/mips/ralink/Platform' as
> > > > cflags.
> > > >
> > > > This driver depends on RALINK but also is enabled for COMPILE_TEST
> > > > where nothing about its platform is known and this directly
> > > > included files are not found at all breaking compilation.
> > > >
> > > > Fix this problem adding include search path for ralink in
> > > > ralink phy directory Makefile.
> > > >
> > > > Fixes: d87da32372a0 ("phy: ralink: Add PHY driver for MT7621 PCIe PHY")
> > >
> > > Pls add reported-by sfr..
> >
> > Ok, Will change this and send v2.
> >
> > >
> > > > Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
> > > > ---
> > > >  drivers/phy/ralink/Makefile | 3 +++
> > > >  1 file changed, 3 insertions(+)
> > > >
> > > > diff --git a/drivers/phy/ralink/Makefile b/drivers/phy/ralink/Makefile
> > > > index cda2a4a7ca5e..c8f9adba0d82 100644
> > > > --- a/drivers/phy/ralink/Makefile
> > > > +++ b/drivers/phy/ralink/Makefile
> > > > @@ -1,3 +1,6 @@
> > > >  # SPDX-License-Identifier: GPL-2.0-only
> > > > +
> > > > +ccflags-y    += -I$(srctree)/arch/mips/include/asm/mach-ralink
> > >
> > > Can we include <asm/mips/...> instead of using this?
> >
> > Doing that also fails because path for 'arch/mips/include' is not
> > added also because we are compiling in x86_64.
> > Looking into other drivers I see the way to avoid this kind of
> > platform specific stuff is adding related paths in
> > cflags. So I don't really know if there is another way.
>
> Right, ideally these headers should have been in include/linux, but.. I
> dont like this way, can we drop compile test..?

Ok, will drop it then and resend.

Thanks,
    Sergio Paracuellos
>
> --
> ~Vinod

[PATCH 2/3] phy: ralink: phy-mt7621-pci: avoid warning if COMPILE_TEST is enabled

[Sergio Paracuellos]

This driver can be enabled for COMPILE_TEST. Casting to unsigned
int for address shows a compilation warning because of size of the
pointer in x86_64 architecture using 'make allmodconfig' target.
Avoid the warning in any architecture casting to unsigned long and
properly setting format also for 'dev_info' macro.

Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 drivers/phy/ralink/phy-mt7621-pci.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/phy/ralink/phy-mt7621-pci.c b/drivers/phy/ralink/phy-mt7621-pci.c
index db79088d5362..befa282e2447 100644
--- a/drivers/phy/ralink/phy-mt7621-pci.c
+++ b/drivers/phy/ralink/phy-mt7621-pci.c
@@ -272,8 +272,8 @@ static struct phy *mt7621_pcie_phy_of_xlate(struct device *dev,
 
 	mt7621_phy->has_dual_port = args->args[0];
 
-	dev_info(dev, "PHY for 0x%08x (dual port = %d)\n",
-		 (unsigned int)mt7621_phy->port_base, mt7621_phy->has_dual_port);
+	dev_info(dev, "PHY for 0x%08lx (dual port = %d)\n",
+		 (unsigned long)mt7621_phy->port_base, mt7621_phy->has_dual_port);
 
 	return mt7621_phy->phy;
 }
-- 
2.25.1

[PATCH 3/3] phy: ralink: phy-mt7621-pci: set correct name in MODULE_DEVICE_TABLE macro

[Sergio Paracuellos]

Correct name passed into 'MODULE_DEVICE_TABLE' which was wrong and
was showing a warning when the driver is enabled for 'COMPILE_TEST'.

Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 drivers/phy/ralink/phy-mt7621-pci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/phy/ralink/phy-mt7621-pci.c b/drivers/phy/ralink/phy-mt7621-pci.c
index befa282e2447..072f848ec0cf 100644
--- a/drivers/phy/ralink/phy-mt7621-pci.c
+++ b/drivers/phy/ralink/phy-mt7621-pci.c
@@ -335,7 +335,7 @@ static const struct of_device_id mt7621_pci_phy_ids[] = {
 	{ .compatible = "mediatek,mt7621-pci-phy" },
 	{},
 };
-MODULE_DEVICE_TABLE(of, mt7621_pci_ids);
+MODULE_DEVICE_TABLE(of, mt7621_pci_phy_ids);
 
 static struct platform_driver mt7621_pci_phy_driver = {
 	.probe = mt7621_pci_phy_probe,
-- 
2.25.1