All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH NEXT 1/4] powerpc/pasemi: Add PCI initialisation for Nemo board.
@ 2017-12-31 22:00 Darren Stevens
  2018-01-03 12:15 ` kbuild test robot
  2018-05-03 13:06 ` Michael Ellerman
  0 siblings, 2 replies; 7+ messages in thread
From: Darren Stevens @ 2017-12-31 22:00 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: chzigotzky

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

  AmigaOS...........: http://yam.ch/
  Unix/MacOS/Windows: http://www.mozilla.com/thunderbird/

General information about MIME can be found at:
http://en.wikipedia.org/wiki/MIME

[-- Attachment #2: Type: text/plain, Size: 524 bytes --]

    The A-Eon Amigaone X1000's Nemo motherboard has an AMD SB600
    connected to one of the PCI-e root ports on its PaSemi
    Pwrficient 1628M SoC. Normally the SB600 southbridge would be
    connected to a hidden PCI-e port on the system's northbridge,
    and as a result doesn't fully comply with the PCI-e spec.
    
    Add code to relax the PCI-e detection in both the root port
    and the Linux kernel allowing on board devices to be detected.
    
    Signed-off-by: Darren Stevens <Darren@stevens-zone.net>

---

[-- Attachment #3: pci.patch --]
[-- Type: text/plain, Size: 5035 bytes --]

diff --git a/arch/powerpc/platforms/pasemi/pasemi.h b/arch/powerpc/platforms/pasemi/pasemi.h
index 329d2a6..8900dee 100644
--- a/arch/powerpc/platforms/pasemi/pasemi.h
+++ b/arch/powerpc/platforms/pasemi/pasemi.h
@@ -3,6 +3,9 @@
 #define _PASEMI_PASEMI_H
 
 extern unsigned long pas_get_boot_time(void);
+#ifdef CONFIG_PPC_PASEMI_NEMO
+extern void nemo_pci_init(void);
+#endif
 extern void pas_pci_init(void);
 extern void pas_pci_irq_fixup(struct pci_dev *dev);
 extern void pas_pci_dma_dev_setup(struct pci_dev *dev);
diff --git a/arch/powerpc/platforms/pasemi/pci.c b/arch/powerpc/platforms/pasemi/pci.c
index 5ff6108..cb0ac87 100644
--- a/arch/powerpc/platforms/pasemi/pci.c
+++ b/arch/powerpc/platforms/pasemi/pci.c
@@ -27,6 +27,7 @@
 #include <linux/pci.h>
 
 #include <asm/pci-bridge.h>
+#include <asm/isa-bridge.h>
 #include <asm/machdep.h>
 
 #include <asm/ppc-pci.h>
@@ -108,6 +109,92 @@ static int workaround_5945(struct pci_bus *bus, unsigned int devfn,
 	return 1;
 }
 
+#ifdef CONFIG_PPC_PASEMI_NEMO
+static int sb600_bus = 5;
+static void __iomem *iob_mapbase = NULL;
+
+static void sb600_set_flag(int bus)
+{
+	struct resource res;
+	struct device_node *dn;
+	int err;
+
+	if (iob_mapbase == NULL) {
+		dn = of_find_compatible_node(NULL, "isa", "pasemi,1682m-iob");
+		if (!dn) {
+			printk(KERN_CRIT "NEMO SB600 missing iob node\n");
+			return;
+		}
+
+		err = of_address_to_resource(dn, 0, &res);
+		of_node_put(dn);
+
+		if (err) {
+			printk(KERN_CRIT "NEMO SB600 missing resource\n");
+			return;
+		}
+
+		printk(KERN_CRIT "NEMO SB600 IOB base %08lx\n",res.start);
+
+		iob_mapbase = ioremap(res.start + 0x100, 0x94);
+	}
+
+	if (iob_mapbase != NULL) {
+		if (bus == sb600_bus) {
+			/*
+			 * This is the SB600's bus, tell the PCI-e root port
+			 * to allow non-zero devices to enumerate.
+			 */
+			out_le32(iob_mapbase + 4, in_le32(iob_mapbase + 4) | 0x800);
+		} else {
+			/*
+			 * Only scan device 0 on other busses
+			 */
+			out_le32(iob_mapbase + 4, in_le32(iob_mapbase + 4) & ~0x800);
+		}
+	}
+}
+
+static int nemo_pxp_read_config(struct pci_bus *bus, unsigned int devfn,
+			      int offset, int len, u32 *val)
+{
+	struct pci_controller *hose;
+	void volatile __iomem *addr;
+
+	hose = pci_bus_to_host(bus);
+	if (!hose)
+		return PCIBIOS_DEVICE_NOT_FOUND;
+
+	if (!pa_pxp_offset_valid(bus->number, devfn, offset))
+		return PCIBIOS_BAD_REGISTER_NUMBER;
+
+	if (workaround_5945(bus, devfn, offset, len, val))
+		return PCIBIOS_SUCCESSFUL;
+
+	addr = pa_pxp_cfg_addr(hose, bus->number, devfn, offset);
+
+	sb600_set_flag(bus->number);
+
+	/*
+	 * Note: the caller has already checked that offset is
+	 * suitably aligned and that len is 1, 2 or 4.
+	 */
+	switch (len) {
+	case 1:
+		*val = in_8(addr);
+		break;
+	case 2:
+		*val = in_le16(addr);
+		break;
+	default:
+		*val = in_le32(addr);
+		break;
+	}
+
+	return PCIBIOS_SUCCESSFUL;
+}
+#endif
+
 static int pa_pxp_read_config(struct pci_bus *bus, unsigned int devfn,
 			      int offset, int len, u32 *val)
 {
@@ -178,6 +265,20 @@ static int pa_pxp_write_config(struct pci_bus *bus, unsigned int devfn,
 	return PCIBIOS_SUCCESSFUL;
 }
 
+#ifdef CONFIG_PPC_PASEMI_NEMO
+static struct pci_ops nemo_pxp_ops = {
+	.read = nemo_pxp_read_config,
+	.write = pa_pxp_write_config,
+};
+
+static void __init setup_nemo_pxp(struct pci_controller *hose)
+{
+	hose->ops = &nemo_pxp_ops;
+	hose->cfg_data = ioremap(0xe0000000, 0x10000000);
+}
+
+#endif
+
 static struct pci_ops pa_pxp_ops = {
 	.read = pa_pxp_read_config,
 	.write = pa_pxp_write_config,
@@ -189,6 +290,35 @@ static void __init setup_pa_pxp(struct pci_controller *hose)
 	hose->cfg_data = ioremap(0xe0000000, 0x10000000);
 }
 
+#ifdef CONFIG_PPC_PASEMI_NEMO
+static int __init nemo_add_bridge(struct device_node *dev)
+{
+	struct pci_controller *hose;
+
+	pr_debug("Adding PCI host bridge %pOF\n", dev);
+
+	hose = pcibios_alloc_controller(dev);
+	if (!hose)
+		return -ENOMEM;
+
+	hose->first_busno = 0;
+	hose->last_busno = 0xff;
+	hose->controller_ops = pasemi_pci_controller_ops;
+
+	setup_nemo_pxp(hose);
+
+	printk(KERN_INFO "Found PA-PXP PCI host bridge.\n");
+
+	/* Interpret the "ranges" property */
+	pci_process_bridge_OF_ranges(hose, dev, 1);
+
+	/* Scan for an isa bridge. */
+	isa_bridge_find_early(hose);
+
+	return 0;
+}
+#endif
+
 static int __init pas_add_bridge(struct device_node *dev)
 {
 	struct pci_controller *hose;
@@ -213,6 +343,28 @@ static int __init pas_add_bridge(struct device_node *dev)
 	return 0;
 }
 
+#ifdef CONFIG_PPC_PASEMI_NEMO
+void __init nemo_pci_init(void)
+{
+	struct device_node *np, *root;
+
+	root = of_find_node_by_path("/");
+	if (!root) {
+		printk(KERN_CRIT "pas_pci_init: can't find root "
+			"of device tree\n");
+		return;
+	}
+
+	pci_set_flags(PCI_SCAN_ALL_PCIE_DEVS);
+
+	for (np = NULL; (np = of_get_next_child(root, np)) != NULL;)
+		if (np->name && !strcmp(np->name, "pxp") && !nemo_add_bridge(np))
+			of_node_get(np);
+
+	of_node_put(root);
+}
+#endif
+
 void __init pas_pci_init(void)
 {
 	struct device_node *np, *root;

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH NEXT 1/4] powerpc/pasemi: Add PCI initialisation for Nemo board.
  2017-12-31 22:00 [PATCH NEXT 1/4] powerpc/pasemi: Add PCI initialisation for Nemo board Darren Stevens
@ 2018-01-03 12:15 ` kbuild test robot
  2018-05-03 13:06 ` Michael Ellerman
  1 sibling, 0 replies; 7+ messages in thread
From: kbuild test robot @ 2018-01-03 12:15 UTC (permalink / raw)
  To: Darren Stevens; +Cc: kbuild-all, linuxppc-dev, chzigotzky

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

Hi Darren,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on powerpc/next]
[also build test WARNING on v4.15-rc6 next-20180103]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Darren-Stevens/powerpc-pasemi-Add-PCI-initialisation-for-Nemo-board/20180103-091349
base:   https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
config: powerpc-allmodconfig (attached as .config)
compiler: powerpc64-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=powerpc 

All warnings (new ones prefixed by >>):

   In file included from include/linux/printk.h:7:0,
                    from include/linux/kernel.h:14,
                    from arch/powerpc/platforms/pasemi/pci.c:26:
   arch/powerpc/platforms/pasemi/pci.c: In function 'sb600_set_flag':
>> include/linux/kern_levels.h:5:18: warning: format '%lx' expects argument of type 'long unsigned int', but argument 2 has type 'resource_size_t {aka long long unsigned int}' [-Wformat=]
    #define KERN_SOH "\001"  /* ASCII Start Of Header */
                     ^
   include/linux/kern_levels.h:10:19: note: in expansion of macro 'KERN_SOH'
    #define KERN_CRIT KERN_SOH "2" /* critical conditions */
                      ^~~~~~~~
>> arch/powerpc/platforms/pasemi/pci.c:137:10: note: in expansion of macro 'KERN_CRIT'
      printk(KERN_CRIT "NEMO SB600 IOB base %08lx\n",res.start);
             ^~~~~~~~~
   arch/powerpc/platforms/pasemi/pci.c:137:45: note: format string is defined here
      printk(KERN_CRIT "NEMO SB600 IOB base %08lx\n",res.start);
                                            ~~~~^
                                            %08llx

vim +/KERN_CRIT +137 arch/powerpc/platforms/pasemi/pci.c

  > 26	#include <linux/kernel.h>
    27	#include <linux/pci.h>
    28	
    29	#include <asm/pci-bridge.h>
    30	#include <asm/isa-bridge.h>
    31	#include <asm/machdep.h>
    32	
    33	#include <asm/ppc-pci.h>
    34	
    35	#include "pasemi.h"
    36	
    37	#define PA_PXP_CFA(bus, devfn, off) (((bus) << 20) | ((devfn) << 12) | (off))
    38	
    39	static inline int pa_pxp_offset_valid(u8 bus, u8 devfn, int offset)
    40	{
    41		/* Device 0 Function 0 is special: It's config space spans function 1 as
    42		 * well, so allow larger offset. It's really a two-function device but the
    43		 * second function does not probe.
    44		 */
    45		if (bus == 0 && devfn == 0)
    46			return offset < 8192;
    47		else
    48			return offset < 4096;
    49	}
    50	
    51	static void volatile __iomem *pa_pxp_cfg_addr(struct pci_controller *hose,
    52					       u8 bus, u8 devfn, int offset)
    53	{
    54		return hose->cfg_data + PA_PXP_CFA(bus, devfn, offset);
    55	}
    56	
    57	static inline int is_root_port(int busno, int devfn)
    58	{
    59		return ((busno == 0) && (PCI_FUNC(devfn) < 4) &&
    60			 ((PCI_SLOT(devfn) == 16) || (PCI_SLOT(devfn) == 17)));
    61	}
    62	
    63	static inline int is_5945_reg(int reg)
    64	{
    65		return (((reg >= 0x18) && (reg < 0x34)) ||
    66			((reg >= 0x158) && (reg < 0x178)));
    67	}
    68	
    69	static int workaround_5945(struct pci_bus *bus, unsigned int devfn,
    70				   int offset, int len, u32 *val)
    71	{
    72		struct pci_controller *hose;
    73		void volatile __iomem *addr, *dummy;
    74		int byte;
    75		u32 tmp;
    76	
    77		if (!is_root_port(bus->number, devfn) || !is_5945_reg(offset))
    78			return 0;
    79	
    80		hose = pci_bus_to_host(bus);
    81	
    82		addr = pa_pxp_cfg_addr(hose, bus->number, devfn, offset & ~0x3);
    83		byte = offset & 0x3;
    84	
    85		/* Workaround bug 5945: write 0 to a dummy register before reading,
    86		 * and write back what we read. We must read/write the full 32-bit
    87		 * contents so we need to shift and mask by hand.
    88		 */
    89		dummy = pa_pxp_cfg_addr(hose, bus->number, devfn, 0x10);
    90		out_le32(dummy, 0);
    91		tmp = in_le32(addr);
    92		out_le32(addr, tmp);
    93	
    94		switch (len) {
    95		case 1:
    96			*val = (tmp >> (8*byte)) & 0xff;
    97			break;
    98		case 2:
    99			if (byte == 0)
   100				*val = tmp & 0xffff;
   101			else
   102				*val = (tmp >> 16) & 0xffff;
   103			break;
   104		default:
   105			*val = tmp;
   106			break;
   107		}
   108	
   109		return 1;
   110	}
   111	
   112	#ifdef CONFIG_PPC_PASEMI_NEMO
   113	static int sb600_bus = 5;
   114	static void __iomem *iob_mapbase = NULL;
   115	
   116	static void sb600_set_flag(int bus)
   117	{
   118		struct resource res;
   119		struct device_node *dn;
   120		int err;
   121	
   122		if (iob_mapbase == NULL) {
   123			dn = of_find_compatible_node(NULL, "isa", "pasemi,1682m-iob");
   124			if (!dn) {
   125				printk(KERN_CRIT "NEMO SB600 missing iob node\n");
   126				return;
   127			}
   128	
   129			err = of_address_to_resource(dn, 0, &res);
   130			of_node_put(dn);
   131	
   132			if (err) {
   133				printk(KERN_CRIT "NEMO SB600 missing resource\n");
   134				return;
   135			}
   136	
 > 137			printk(KERN_CRIT "NEMO SB600 IOB base %08lx\n",res.start);
   138	
   139			iob_mapbase = ioremap(res.start + 0x100, 0x94);
   140		}
   141	
   142		if (iob_mapbase != NULL) {
   143			if (bus == sb600_bus) {
   144				/*
   145				 * This is the SB600's bus, tell the PCI-e root port
   146				 * to allow non-zero devices to enumerate.
   147				 */
   148				out_le32(iob_mapbase + 4, in_le32(iob_mapbase + 4) | 0x800);
   149			} else {
   150				/*
   151				 * Only scan device 0 on other busses
   152				 */
   153				out_le32(iob_mapbase + 4, in_le32(iob_mapbase + 4) & ~0x800);
   154			}
   155		}
   156	}
   157	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 56161 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH NEXT 1/4] powerpc/pasemi: Add PCI initialisation for Nemo board.
  2017-12-31 22:00 [PATCH NEXT 1/4] powerpc/pasemi: Add PCI initialisation for Nemo board Darren Stevens
  2018-01-03 12:15 ` kbuild test robot
@ 2018-05-03 13:06 ` Michael Ellerman
  2018-07-10  5:43   ` Christian Zigotzky
  1 sibling, 1 reply; 7+ messages in thread
From: Michael Ellerman @ 2018-05-03 13:06 UTC (permalink / raw)
  To: Darren Stevens, linuxppc-dev; +Cc: chzigotzky

Hi Darren,

Thanks for the patch, sorry it's taken so long to get merged.

Darren Stevens <darren@stevens-zone.net> writes:
>     The A-Eon Amigaone X1000's Nemo motherboard has an AMD SB600
>     connected to one of the PCI-e root ports on its PaSemi
>     Pwrficient 1628M SoC. Normally the SB600 southbridge would be
>     connected to a hidden PCI-e port on the system's northbridge,
>     and as a result doesn't fully comply with the PCI-e spec.
>     
>     Add code to relax the PCI-e detection in both the root port
>     and the Linux kernel allowing on board devices to be detected.
>     
>     Signed-off-by: Darren Stevens <Darren@stevens-zone.net>

This should not be indented.

> diff --git a/arch/powerpc/platforms/pasemi/pasemi.h b/arch/powerpc/platforms/pasemi/pasemi.h
> index 329d2a6..8900dee 100644
> --- a/arch/powerpc/platforms/pasemi/pasemi.h
> +++ b/arch/powerpc/platforms/pasemi/pasemi.h
> @@ -3,6 +3,9 @@
>  #define _PASEMI_PASEMI_H
>  
>  extern unsigned long pas_get_boot_time(void);
> +#ifdef CONFIG_PPC_PASEMI_NEMO
> +extern void nemo_pci_init(void);
> +#endif

We don't need an #ifdef around this thanks.

> diff --git a/arch/powerpc/platforms/pasemi/pci.c b/arch/powerpc/platforms/pasemi/pci.c
> index 5ff6108..cb0ac87 100644
> --- a/arch/powerpc/platforms/pasemi/pci.c
> +++ b/arch/powerpc/platforms/pasemi/pci.c
> @@ -108,6 +109,92 @@ static int workaround_5945(struct pci_bus *bus, unsigned int devfn,
>  	return 1;
>  }
>  
> +#ifdef CONFIG_PPC_PASEMI_NEMO
> +static int sb600_bus = 5;

That's only used once so could just be a #define, or even a literal in
the code.

> +static void __iomem *iob_mapbase = NULL;

That's only used in sb6000_set_flag() so should be in there shouldn't it?

> +static void sb600_set_flag(int bus)
> +{
> +	struct resource res;
> +	struct device_node *dn;
> +	int err;
> +
> +	if (iob_mapbase == NULL) {
> +		dn = of_find_compatible_node(NULL, "isa", "pasemi,1682m-iob");
> +		if (!dn) {
> +			printk(KERN_CRIT "NEMO SB600 missing iob node\n");

I'm not sure CRIT is really necessary, we tend to use ERR, but I don't
mind that much.

But can you use pr_err()/pr_crit() please.

> +			return;
> +		}
> +
> +		err = of_address_to_resource(dn, 0, &res);
> +		of_node_put(dn);
> +
> +		if (err) {
> +			printk(KERN_CRIT "NEMO SB600 missing resource\n");
> +			return;
> +		}
> +
> +		printk(KERN_CRIT "NEMO SB600 IOB base %08lx\n",res.start);

That's INFO or even DEBUG.

> +
> +		iob_mapbase = ioremap(res.start + 0x100, 0x94);

0x94 ?

It's going to map you at least one page anyway.

> +	}
> +
> +	if (iob_mapbase != NULL) {
> +		if (bus == sb600_bus) {
> +			/*
> +			 * This is the SB600's bus, tell the PCI-e root port
> +			 * to allow non-zero devices to enumerate.
> +			 */
> +			out_le32(iob_mapbase + 4, in_le32(iob_mapbase + 4) | 0x800);

Does reg #4 have a name?
Or 0x800 ?

> +		} else {
> +			/*
> +			 * Only scan device 0 on other busses
> +			 */
> +			out_le32(iob_mapbase + 4, in_le32(iob_mapbase + 4) & ~0x800);
> +		}
> +	}
> +}
> +
> +static int nemo_pxp_read_config(struct pci_bus *bus, unsigned int devfn,
> +			      int offset, int len, u32 *val)
> +{
> +	struct pci_controller *hose;

Can we call them host or controller or something in new code?

> +	void volatile __iomem *addr;

Does that need to be volatile?

> +	hose = pci_bus_to_host(bus);
> +	if (!hose)
> +		return PCIBIOS_DEVICE_NOT_FOUND;
> +
> +	if (!pa_pxp_offset_valid(bus->number, devfn, offset))
> +		return PCIBIOS_BAD_REGISTER_NUMBER;
> +
> +	if (workaround_5945(bus, devfn, offset, len, val))
> +		return PCIBIOS_SUCCESSFUL;
> +
> +	addr = pa_pxp_cfg_addr(hose, bus->number, devfn, offset);
> +
> +	sb600_set_flag(bus->number);
> +
> +	/*
> +	 * Note: the caller has already checked that offset is
> +	 * suitably aligned and that len is 1, 2 or 4.
> +	 */
> +	switch (len) {
> +	case 1:
> +		*val = in_8(addr);
> +		break;
> +	case 2:
> +		*val = in_le16(addr);
> +		break;
> +	default:
> +		*val = in_le32(addr);
> +		break;
> +	}
> +
> +	return PCIBIOS_SUCCESSFUL;
> +}
> +#endif
> +
>  static int pa_pxp_read_config(struct pci_bus *bus, unsigned int devfn,
>  			      int offset, int len, u32 *val)
>  {
> @@ -178,6 +265,20 @@ static int pa_pxp_write_config(struct pci_bus *bus, unsigned int devfn,
>  	return PCIBIOS_SUCCESSFUL;
>  }
>  
> +#ifdef CONFIG_PPC_PASEMI_NEMO
> +static struct pci_ops nemo_pxp_ops = {
> +	.read = nemo_pxp_read_config,
> +	.write = pa_pxp_write_config,
> +};
> +
> +static void __init setup_nemo_pxp(struct pci_controller *hose)
> +{
> +	hose->ops = &nemo_pxp_ops;
> +	hose->cfg_data = ioremap(0xe0000000, 0x10000000);
> +}

Could these go in one of the ifdef block below? Just to reduce the
number of times we ifdef NEMO.
> @@ -213,6 +343,28 @@ static int __init pas_add_bridge(struct device_node *dev)
>  	return 0;
>  }
>  
> +#ifdef CONFIG_PPC_PASEMI_NEMO
> +void __init nemo_pci_init(void)
> +{
> +	struct device_node *np, *root;
> +
> +	root = of_find_node_by_path("/");
> +	if (!root) {
> +		printk(KERN_CRIT "pas_pci_init: can't find root "
> +			"of device tree\n");

TBH that can't really happen, or if it does we're not going to boot
elsewhere.

So the printk() is probably overkill.

> +		return;
> +	}
> +
> +	pci_set_flags(PCI_SCAN_ALL_PCIE_DEVS);
> +
> +	for (np = NULL; (np = of_get_next_child(root, np)) != NULL;)

Does the pxp node not have a compatible property? (I may have asked that
before).

Normally you'd search by compatible, not name.

If you have to search by name then of_get_child_by_name() should work.

> +		if (np->name && !strcmp(np->name, "pxp") && !nemo_add_bridge(np))
> +			of_node_get(np);

Why are we of_node_get()'ing here?

If nemo_add_bridge() wants to keep a reference it should do that itself.

> +
> +	of_node_put(root);
> +}
> +#endif
> +
>  void __init pas_pci_init(void)
>  {
>  	struct device_node *np, *root;


cheers

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH NEXT 1/4] powerpc/pasemi: Add PCI initialisation for Nemo board.
  2018-05-03 13:06 ` Michael Ellerman
@ 2018-07-10  5:43   ` Christian Zigotzky
  2018-07-10 13:50     ` Michael Ellerman
  0 siblings, 1 reply; 7+ messages in thread
From: Christian Zigotzky @ 2018-07-10  5:43 UTC (permalink / raw)
  To: Michael Ellerman; +Cc: Darren Stevens, linuxppc-dev

Hi Michael,
Hi All,

kbuild test robot Wed, 03 Jan 2018 04:17:20 -0800 wrote:

Hi Darren,

Thank you for the patch! Perhaps something to improve:

arch/powerpc/platforms/pasemi/pci.c: In function 'sb600_set_flag': >> includ=
e/linux/kern_levels.h:5:18: warning: format '%lx' expects argument of >> typ=
e 'long unsigned int', but argument 2 has type 'resource_size_t {aka long >>=
 long unsigned int}' [-Wformat=3D] #define KERN_SOH "\001" /* ASCII Start Of=
 Header */

=E2=80=94-

I was able to fix this small format issue. I replaced the format '%08lx' wit=
h '%08llx'.

+ printk(KERN_CRIT "NEMO SB600 IOB base %08llx\n",res.start);

Is this fix OK or is there a better solution?

> On 3. May 2018, at 15:06, Michael Ellerman <mpe@ellerman.id.au> wrote:
>=20
>> +
>> +        printk(KERN_CRIT "NEMO SB600 IOB base %08lx\n",res.start);
>=20
> That's INFO or even DEBUG.
>>=20

Michael,

What do you think about this fix?

+ printk(KERN_INFO "NEMO SB600 IOB base %08llx\n",res.start);

Thanks,
Christian=

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH NEXT 1/4] powerpc/pasemi: Add PCI initialisation for Nemo board.
  2018-07-10  5:43   ` Christian Zigotzky
@ 2018-07-10 13:50     ` Michael Ellerman
  2018-07-10 17:15       ` Christian Zigotzky
  0 siblings, 1 reply; 7+ messages in thread
From: Michael Ellerman @ 2018-07-10 13:50 UTC (permalink / raw)
  To: Christian Zigotzky; +Cc: Darren Stevens, linuxppc-dev

Christian Zigotzky <chzigotzky@xenosoft.de> writes:

> Hi Michael,
> Hi All,
>
> kbuild test robot Wed, 03 Jan 2018 04:17:20 -0800 wrote:
>
> Hi Darren,
>
> Thank you for the patch! Perhaps something to improve:
>
> arch/powerpc/platforms/pasemi/pci.c: In function 'sb600_set_flag': >> inc=
lude/linux/kern_levels.h:5:18: warning: format '%lx' expects argument of >>=
 type 'long unsigned int', but argument 2 has type 'resource_size_t {aka lo=
ng >> long unsigned int}' [-Wformat=3D] #define KERN_SOH "\001" /* ASCII St=
art Of Header */
>
> =E2=80=94-
>
> I was able to fix this small format issue. I replaced the format '%08lx' =
with '%08llx'.
>
> + printk(KERN_CRIT "NEMO SB600 IOB base %08llx\n",res.start);
>
> Is this fix OK or is there a better solution?
>
>> On 3. May 2018, at 15:06, Michael Ellerman <mpe@ellerman.id.au> wrote:
>>=20
>>> +
>>> +        printk(KERN_CRIT "NEMO SB600 IOB base %08lx\n",res.start);
>>=20
>> That's INFO or even DEBUG.
>>>=20
>
> Michael,
>
> What do you think about this fix?
>
> + printk(KERN_INFO "NEMO SB600 IOB base %08llx\n",res.start);

pr_info() would be nice.

But I replied with lots of other comments previously.

None of them were super critical, but it would be nice to get them fixed
before merging if possible.

cheers

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH NEXT 1/4] powerpc/pasemi: Add PCI initialisation for Nemo board.
  2018-07-10 13:50     ` Michael Ellerman
@ 2018-07-10 17:15       ` Christian Zigotzky
  2018-07-11 14:04         ` Michael Ellerman
  0 siblings, 1 reply; 7+ messages in thread
From: Christian Zigotzky @ 2018-07-10 17:15 UTC (permalink / raw)
  To: Michael Ellerman; +Cc: Darren Stevens, linuxppc-dev

Hello Michael,

Thanks a lot for your reply. OK, first I would like to add=20

pr_info("NEMO SB600 IOB base %08llx\n",res.start)

to the Nemo patch. Is this line correct now?

After that I will try to contact Darren because of your other comments.

If I don=E2=80=99t reach Darren then I will try to fix the issues but I thin=
k I need your help for fixing them.

Thanks,
Christian

> On 10. Jul 2018, at 15:50, Michael Ellerman wrote:
>=20
> pr_info() would be nice.
>=20
> But I replied with lots of other comments previously.
>=20
> None of them were super critical, but it would be nice to get them fixed
> before merging if possible.
>=20
> cheers

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH NEXT 1/4] powerpc/pasemi: Add PCI initialisation for Nemo board.
  2018-07-10 17:15       ` Christian Zigotzky
@ 2018-07-11 14:04         ` Michael Ellerman
  0 siblings, 0 replies; 7+ messages in thread
From: Michael Ellerman @ 2018-07-11 14:04 UTC (permalink / raw)
  To: Christian Zigotzky; +Cc: Darren Stevens, linuxppc-dev

Christian Zigotzky <chzigotzky@xenosoft.de> writes:

> Hello Michael,
>
> Thanks a lot for your reply. OK, first I would like to add=20
>
> pr_info("NEMO SB600 IOB base %08llx\n",res.start)
>
> to the Nemo patch. Is this line correct now?

Yes I think so.

The type of start is resource_size_t, which can be 32 or 64-bits. But in
this code it should always be 64-bit.

> After that I will try to contact Darren because of your other comments.
>
> If I don=E2=80=99t reach Darren then I will try to fix the issues but I t=
hink
> I need your help for fixing them.

Sure.

cheers

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2018-07-11 14:04 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-12-31 22:00 [PATCH NEXT 1/4] powerpc/pasemi: Add PCI initialisation for Nemo board Darren Stevens
2018-01-03 12:15 ` kbuild test robot
2018-05-03 13:06 ` Michael Ellerman
2018-07-10  5:43   ` Christian Zigotzky
2018-07-10 13:50     ` Michael Ellerman
2018-07-10 17:15       ` Christian Zigotzky
2018-07-11 14:04         ` Michael Ellerman

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.