LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 2/3 v3] mtd/powerpc: Factor out MTD physmap bindings into mtd-physmap.txt
From: Stefan Roese @ 2009-04-16 12:10 UTC (permalink / raw)
  To: linuxppc-dev, linux-mtd; +Cc: devicetree-discuss

It's easier to find bindings descriptions in separate files. So let's factor out
the MTD physmap bindings into Documentation/powerpc/dts-bindings/mtd-physmap.txt
to not clutter booting-without-of.txt more.

Signed-off-by: Stefan Roese <sr@denx.de>
Acked-by: Grant Likely <grant.likely@secretlab.ca>
---
Changes in ver3:
- Added short patch desciption to commit message.

 Documentation/powerpc/booting-without-of.txt       |   89 +++-----------------
 Documentation/powerpc/dts-bindings/mtd-physmap.txt |   63 ++++++++++++++
 2 files changed, 75 insertions(+), 77 deletions(-)
 create mode 100644 Documentation/powerpc/dts-bindings/mtd-physmap.txt

diff --git a/Documentation/powerpc/booting-without-of.txt b/Documentation/powerpc/booting-without-of.txt
index 0ab0230..d16b7a1 100644
--- a/Documentation/powerpc/booting-without-of.txt
+++ b/Documentation/powerpc/booting-without-of.txt
@@ -43,12 +43,11 @@ Table of Contents
     2) Representing devices without a current OF specification
       a) PHY nodes
       b) Interrupt controllers
-      c) CFI or JEDEC memory-mapped NOR flash
-      d) 4xx/Axon EMAC ethernet nodes
-      e) Xilinx IP cores
-      f) USB EHCI controllers
-      g) MDIO on GPIOs
-      h) SPI busses
+      c) 4xx/Axon EMAC ethernet nodes
+      d) Xilinx IP cores
+      e) USB EHCI controllers
+      f) MDIO on GPIOs
+      g) SPI busses
 
   VII - Marvell Discovery mv64[345]6x System Controller chips
     1) The /system-controller node
@@ -999,7 +998,7 @@ compatibility.
       translation of SOC addresses for memory mapped SOC registers.
     - bus-frequency: Contains the bus frequency for the SOC node.
       Typically, the value of this field is filled in by the boot
-      loader. 
+      loader.
 
 
   Recommended properties:
@@ -1287,71 +1286,7 @@ platforms are moved over to use the flattened-device-tree model.
 		device_type = "open-pic";
 	};
 
-   c) CFI or JEDEC memory-mapped NOR flash
-
-    Flash chips (Memory Technology Devices) are often used for solid state
-    file systems on embedded devices.
-
-     - compatible : should contain the specific model of flash chip(s)
-       used, if known, followed by either "cfi-flash" or "jedec-flash"
-     - reg : Address range of the flash chip
-     - bank-width : Width (in bytes) of the flash bank.  Equal to the
-       device width times the number of interleaved chips.
-     - device-width : (optional) Width of a single flash chip.  If
-       omitted, assumed to be equal to 'bank-width'.
-     - #address-cells, #size-cells : Must be present if the flash has
-       sub-nodes representing partitions (see below).  In this case
-       both #address-cells and #size-cells must be equal to 1.
-
-    For JEDEC compatible devices, the following additional properties
-    are defined:
-
-     - vendor-id : Contains the flash chip's vendor id (1 byte).
-     - device-id : Contains the flash chip's device id (1 byte).
-
-    In addition to the information on the flash bank itself, the
-    device tree may optionally contain additional information
-    describing partitions of the flash address space.  This can be
-    used on platforms which have strong conventions about which
-    portions of the flash are used for what purposes, but which don't
-    use an on-flash partition table such as RedBoot.
-
-    Each partition is represented as a sub-node of the flash device.
-    Each node's name represents the name of the corresponding
-    partition of the flash device.
-
-    Flash partitions
-     - reg : The partition's offset and size within the flash bank.
-     - label : (optional) The label / name for this flash partition.
-       If omitted, the label is taken from the node name (excluding
-       the unit address).
-     - read-only : (optional) This parameter, if present, is a hint to
-       Linux that this flash partition should only be mounted
-       read-only.  This is usually used for flash partitions
-       containing early-boot firmware images or data which should not
-       be clobbered.
-
-    Example:
-
-	flash@ff000000 {
-		compatible = "amd,am29lv128ml", "cfi-flash";
-		reg = <ff000000 01000000>;
-		bank-width = <4>;
-		device-width = <1>;
-		#address-cells = <1>;
-		#size-cells = <1>;
-		fs@0 {
-			label = "fs";
-			reg = <0 f80000>;
-		};
-		firmware@f80000 {
-			label ="firmware";
-			reg = <f80000 80000>;
-			read-only;
-		};
-	};
-
-    d) 4xx/Axon EMAC ethernet nodes
+    c) 4xx/Axon EMAC ethernet nodes
 
     The EMAC ethernet controller in IBM and AMCC 4xx chips, and also
     the Axon bridge.  To operate this needs to interact with a ths
@@ -1499,7 +1434,7 @@ platforms are moved over to use the flattened-device-tree model.
 			   available.
 			   For Axon: 0x0000012a
 
-   e) Xilinx IP cores
+   d) Xilinx IP cores
 
    The Xilinx EDK toolchain ships with a set of IP cores (devices) for use
    in Xilinx Spartan and Virtex FPGAs.  The devices cover the whole range
@@ -1761,7 +1696,7 @@ platforms are moved over to use the flattened-device-tree model.
       listed above, nodes for these devices should include a phy-handle
       property, and may include other common network device properties
       like local-mac-address.
-      
+
       iv) Xilinx Uartlite
 
       Xilinx uartlite devices are simple fixed speed serial ports.
@@ -1793,7 +1728,7 @@ platforms are moved over to use the flattened-device-tree model.
        - reg-offset : A value of 3 is required
        - reg-shift : A value of 2 is required
 
-    f) USB EHCI controllers
+    e) USB EHCI controllers
 
     Required properties:
       - compatible : should be "usb-ehci".
@@ -1819,7 +1754,7 @@ platforms are moved over to use the flattened-device-tree model.
 		   big-endian;
 	   };
 
-   g) MDIO on GPIOs
+   f) MDIO on GPIOs
 
    Currently defined compatibles:
    - virtual,gpio-mdio
@@ -1839,7 +1774,7 @@ platforms are moved over to use the flattened-device-tree model.
 			 &qe_pio_c 6>;
 	};
 
-    h) SPI (Serial Peripheral Interface) busses
+    g) SPI (Serial Peripheral Interface) busses
 
     SPI busses can be described with a node for the SPI master device
     and a set of child nodes for each SPI slave on the bus.  For this
diff --git a/Documentation/powerpc/dts-bindings/mtd-physmap.txt b/Documentation/powerpc/dts-bindings/mtd-physmap.txt
new file mode 100644
index 0000000..cd474f9
--- /dev/null
+++ b/Documentation/powerpc/dts-bindings/mtd-physmap.txt
@@ -0,0 +1,63 @@
+CFI or JEDEC memory-mapped NOR flash
+
+Flash chips (Memory Technology Devices) are often used for solid state
+file systems on embedded devices.
+
+ - compatible : should contain the specific model of flash chip(s)
+   used, if known, followed by either "cfi-flash" or "jedec-flash"
+ - reg : Address range of the flash chip
+ - bank-width : Width (in bytes) of the flash bank.  Equal to the
+   device width times the number of interleaved chips.
+ - device-width : (optional) Width of a single flash chip.  If
+   omitted, assumed to be equal to 'bank-width'.
+ - #address-cells, #size-cells : Must be present if the flash has
+   sub-nodes representing partitions (see below).  In this case
+   both #address-cells and #size-cells must be equal to 1.
+
+For JEDEC compatible devices, the following additional properties
+are defined:
+
+ - vendor-id : Contains the flash chip's vendor id (1 byte).
+ - device-id : Contains the flash chip's device id (1 byte).
+
+In addition to the information on the flash bank itself, the
+device tree may optionally contain additional information
+describing partitions of the flash address space.  This can be
+used on platforms which have strong conventions about which
+portions of the flash are used for what purposes, but which don't
+use an on-flash partition table such as RedBoot.
+
+Each partition is represented as a sub-node of the flash device.
+Each node's name represents the name of the corresponding
+partition of the flash device.
+
+Flash partitions
+ - reg : The partition's offset and size within the flash bank.
+ - label : (optional) The label / name for this flash partition.
+   If omitted, the label is taken from the node name (excluding
+   the unit address).
+ - read-only : (optional) This parameter, if present, is a hint to
+   Linux that this flash partition should only be mounted
+   read-only.  This is usually used for flash partitions
+   containing early-boot firmware images or data which should not
+   be clobbered.
+
+Example:
+
+	flash@ff000000 {
+		compatible = "amd,am29lv128ml", "cfi-flash";
+		reg = <ff000000 01000000>;
+		bank-width = <4>;
+		device-width = <1>;
+		#address-cells = <1>;
+		#size-cells = <1>;
+		fs@0 {
+			label = "fs";
+			reg = <0 f80000>;
+		};
+		firmware@f80000 {
+			label ="firmware";
+			reg = <f80000 80000>;
+			read-only;
+		};
+	};
-- 
1.6.2.3

^ permalink raw reply related

* [PATCH 1/3 v3] mtd: physmap_of: Add multiple regions and concatenation support
From: Stefan Roese @ 2009-04-16 12:10 UTC (permalink / raw)
  To: linuxppc-dev, linux-mtd; +Cc: devicetree-discuss

This patch adds support to handle multiple non-identical chips in one
flash device tree node. It also adds concat support to physmap_of. This
makes it possible to support e.g. the Intel P30 48F4400 chips which
internally consists of 2 non-identical NOR chips on one die. Additionally
partitions now can span over multiple chips.

To describe such a chip's, multiple "reg" tuples are now supported in one
flash device tree node. Here an dts example:

        flash@f0000000,0 {
                #address-cells = <1>;
                #size-cells = <1>;
                compatible = "cfi-flash";
                reg = <0 0x00000000 0x02000000
                       0 0x02000000 0x02000000>;
                bank-width = <2>;
                partition@0 {
                        label = "test-part1";
                        reg = <0 0x04000000>;
                };
        };

Signed-off-by: Stefan Roese <sr@denx.de>
Reviewd-by: Grant Likely <grant.likely@secretlab.ca>
---
Changes in ver3:
- s/4/sizeof(u32)

Changes in ver2 (as suggested by Grant Likely):
- Removed MAX_RESOURCES introduced in ver1. Now we don't have a hard limit
  for "reg" tuples anymore.
- Used of_n_addr_cells() and of_n_size_cells() to determine size of each tuple.

 drivers/mtd/maps/physmap_of.c |  199 +++++++++++++++++++++++++++++------------
 1 files changed, 143 insertions(+), 56 deletions(-)

diff --git a/drivers/mtd/maps/physmap_of.c b/drivers/mtd/maps/physmap_of.c
index c83a60f..39d357b 100644
--- a/drivers/mtd/maps/physmap_of.c
+++ b/drivers/mtd/maps/physmap_of.c
@@ -20,16 +20,23 @@
 #include <linux/mtd/mtd.h>
 #include <linux/mtd/map.h>
 #include <linux/mtd/partitions.h>
+#include <linux/mtd/concat.h>
 #include <linux/of.h>
 #include <linux/of_platform.h>
 
+struct of_flash_list {
+	struct mtd_info *mtd;
+	struct map_info map;
+	struct resource *res;
+};
+
 struct of_flash {
-	struct mtd_info		*mtd;
-	struct map_info		map;
-	struct resource		*res;
+	struct mtd_info		*cmtd;
 #ifdef CONFIG_MTD_PARTITIONS
 	struct mtd_partition	*parts;
 #endif
+	int list_size; /* number of elements in of_flash_list */
+	struct of_flash_list	list[0];
 };
 
 #ifdef CONFIG_MTD_PARTITIONS
@@ -88,30 +95,44 @@ static int parse_obsolete_partitions(struct of_device *dev,
 static int of_flash_remove(struct of_device *dev)
 {
 	struct of_flash *info;
+	int i;
 
 	info = dev_get_drvdata(&dev->dev);
 	if (!info)
 		return 0;
 	dev_set_drvdata(&dev->dev, NULL);
 
-	if (info->mtd) {
+#ifdef CONFIG_MTD_CONCAT
+	if (info->cmtd != info->list[0].mtd) {
+		del_mtd_device(info->cmtd);
+		mtd_concat_destroy(info->cmtd);
+	}
+#endif
+
+	if (info->cmtd) {
 		if (OF_FLASH_PARTS(info)) {
-			del_mtd_partitions(info->mtd);
+			del_mtd_partitions(info->cmtd);
 			kfree(OF_FLASH_PARTS(info));
 		} else {
-			del_mtd_device(info->mtd);
+			del_mtd_device(info->cmtd);
 		}
-		map_destroy(info->mtd);
 	}
 
-	if (info->map.virt)
-		iounmap(info->map.virt);
+	for (i = 0; i < info->list_size; i++) {
+		if (info->list[i].mtd)
+			map_destroy(info->list[i].mtd);
 
-	if (info->res) {
-		release_resource(info->res);
-		kfree(info->res);
+		if (info->list[i].map.virt)
+			iounmap(info->list[i].map.virt);
+
+		if (info->list[i].res) {
+			release_resource(info->list[i].res);
+			kfree(info->list[i].res);
+		}
 	}
 
+	kfree(info);
+
 	return 0;
 }
 
@@ -164,68 +185,130 @@ static int __devinit of_flash_probe(struct of_device *dev,
 	const char *probe_type = match->data;
 	const u32 *width;
 	int err;
-
-	err = -ENXIO;
-	if (of_address_to_resource(dp, 0, &res)) {
-		dev_err(&dev->dev, "Can't get IO address from device tree\n");
+	int i;
+	int count;
+	const u32 *p;
+	int reg_tuple_size;
+	struct mtd_info **mtd_list = NULL;
+
+	reg_tuple_size = (of_n_addr_cells(dp) + of_n_size_cells(dp)) * sizeof(u32);
+
+	/*
+	 * Get number of "reg" tuples. Scan for MTD devices on area's
+	 * described by each "reg" region. This makes it possible (including
+	 * the concat support) to support the Intel P30 48F4400 chips which
+	 * consists internally of 2 non-identical NOR chips on one die.
+	 */
+	p = of_get_property(dp, "reg", &count);
+	if (count % reg_tuple_size != 0) {
+		dev_err(&dev->dev, "Malformed reg property on %s\n",
+				dev->node->full_name);
+		err = -EINVAL;
 		goto err_out;
 	}
-
-       	dev_dbg(&dev->dev, "of_flash device: %.8llx-%.8llx\n",
-		(unsigned long long)res.start, (unsigned long long)res.end);
+	count /= reg_tuple_size;
 
 	err = -ENOMEM;
-	info = kzalloc(sizeof(*info), GFP_KERNEL);
+	info = kzalloc(sizeof(struct of_flash) +
+		       sizeof(struct of_flash_list) * count, GFP_KERNEL);
+	if (!info)
+		goto err_out;
+
+	mtd_list = kzalloc(sizeof(struct mtd_info) * count, GFP_KERNEL);
 	if (!info)
 		goto err_out;
 
 	dev_set_drvdata(&dev->dev, info);
 
-	err = -EBUSY;
-	info->res = request_mem_region(res.start, res.end - res.start + 1,
-				       dev_name(&dev->dev));
-	if (!info->res)
-		goto err_out;
+	for (i = 0; i < count; i++) {
+		err = -ENXIO;
+		if (of_address_to_resource(dp, i, &res)) {
+			dev_err(&dev->dev, "Can't get IO address from device"
+				" tree\n");
+			goto err_out;
+		}
 
-	err = -ENXIO;
-	width = of_get_property(dp, "bank-width", NULL);
-	if (!width) {
-		dev_err(&dev->dev, "Can't get bank width from device tree\n");
-		goto err_out;
-	}
+		dev_dbg(&dev->dev, "of_flash device: %.8llx-%.8llx\n",
+			(unsigned long long)res.start,
+			(unsigned long long)res.end);
+
+		err = -EBUSY;
+		info->list[i].res = request_mem_region(res.start, res.end -
+						       res.start + 1,
+						       dev_name(&dev->dev));
+		if (!info->list[i].res)
+			goto err_out;
+
+		err = -ENXIO;
+		width = of_get_property(dp, "bank-width", NULL);
+		if (!width) {
+			dev_err(&dev->dev, "Can't get bank width from device"
+				" tree\n");
+			goto err_out;
+		}
 
-	info->map.name = dev_name(&dev->dev);
-	info->map.phys = res.start;
-	info->map.size = res.end - res.start + 1;
-	info->map.bankwidth = *width;
+		info->list[i].map.name = dev_name(&dev->dev);
+		info->list[i].map.phys = res.start;
+		info->list[i].map.size = res.end - res.start + 1;
+		info->list[i].map.bankwidth = *width;
+
+		err = -ENOMEM;
+		info->list[i].map.virt = ioremap(info->list[i].map.phys,
+						 info->list[i].map.size);
+		if (!info->list[i].map.virt) {
+			dev_err(&dev->dev, "Failed to ioremap() flash"
+				" region\n");
+			goto err_out;
+		}
 
-	err = -ENOMEM;
-	info->map.virt = ioremap(info->map.phys, info->map.size);
-	if (!info->map.virt) {
-		dev_err(&dev->dev, "Failed to ioremap() flash region\n");
-		goto err_out;
-	}
+		simple_map_init(&info->list[i].map);
 
-	simple_map_init(&info->map);
+		if (probe_type) {
+			info->list[i].mtd = do_map_probe(probe_type,
+							 &info->list[i].map);
+		} else {
+			info->list[i].mtd = obsolete_probe(dev,
+							   &info->list[i].map);
+		}
+		mtd_list[i] = info->list[i].mtd;
 
-	if (probe_type)
-		info->mtd = do_map_probe(probe_type, &info->map);
-	else
-		info->mtd = obsolete_probe(dev, &info->map);
+		err = -ENXIO;
+		if (!info->list[i].mtd) {
+			dev_err(&dev->dev, "do_map_probe() failed\n");
+			goto err_out;
+		} else {
+			info->list_size++;
+		}
+		info->list[i].mtd->owner = THIS_MODULE;
+		info->list[i].mtd->dev.parent = &dev->dev;
+	}
 
-	err = -ENXIO;
-	if (!info->mtd) {
-		dev_err(&dev->dev, "do_map_probe() failed\n");
-		goto err_out;
+	err = 0;
+	if (info->list_size == 1) {
+		info->cmtd = info->list[0].mtd;
+	} else if (info->list_size > 1) {
+		/*
+		 * We detected multiple devices. Concatenate them together.
+		 */
+#ifdef CONFIG_MTD_CONCAT
+		info->cmtd = mtd_concat_create(mtd_list, info->list_size,
+					       dev_name(&dev->dev));
+		if (info->cmtd == NULL)
+			err = -ENXIO;
+#else
+		printk(KERN_ERR "physmap_of: multiple devices "
+		       "found but MTD concat support disabled.\n");
+		err = -ENXIO;
+#endif
 	}
-	info->mtd->owner = THIS_MODULE;
-	info->mtd->dev.parent = &dev->dev;
+	if (err)
+		goto err_out;
 
 #ifdef CONFIG_MTD_PARTITIONS
 	/* First look for RedBoot table or partitions on the command
 	 * line, these take precedence over device tree information */
-	err = parse_mtd_partitions(info->mtd, part_probe_types,
-	                           &info->parts, 0);
+	err = parse_mtd_partitions(info->cmtd, part_probe_types,
+				   &info->parts, 0);
 	if (err < 0)
 		return err;
 
@@ -244,15 +327,19 @@ static int __devinit of_flash_probe(struct of_device *dev,
 	}
 
 	if (err > 0)
-		add_mtd_partitions(info->mtd, info->parts, err);
+		add_mtd_partitions(info->cmtd, info->parts, err);
 	else
 #endif
-		add_mtd_device(info->mtd);
+		add_mtd_device(info->cmtd);
+
+	kfree(mtd_list);
 
 	return 0;
 
 err_out:
+	kfree(mtd_list);
 	of_flash_remove(dev);
+
 	return err;
 }
 
-- 
1.6.2.3

^ permalink raw reply related

* [PATCH 0/3 v3] mtd: physmap_of: Add multiple regions and concatenation support
From: Stefan Roese @ 2009-04-16 12:05 UTC (permalink / raw)
  To: linux-mtd, linuxppc-dev; +Cc: devicetree-discuss

This patchset adds support to handle multiple non-identical chips in one
flash device tree node. It also adds concat support to physmap_of. This
makes it possible to support e.g. the Intel P30 48F4400 chip which
internally consists of 2 non-identical NOR chips on one die. Additionally
partitions now can span over multiple chips:

    mtd: physmap_of: Add multiple regions and concatenation support
    mtd/powerpc: Factor out MTD physmap bindings into mtd-physmap.txt
    mtd/powerpc: Describe multiple "reg" tuples usage

v2 addresses all comments from Grant Likely, including factoring out the
MTD dts bindings documentation into a separate file.

v3 addresses all comments (again from Grant).

Thanks,
Stefan

^ permalink raw reply

* Re: [ppc64] 2.6.29-git7 : offlining a cpu causes an exception
From: Sachin Sant @ 2009-04-16 10:15 UTC (permalink / raw)
  To: michael; +Cc: linuxppc-dev
In-Reply-To: <1239870306.14642.181.camel@localhost>

Michael Ellerman wrote:
> Does this patch, on top of Ben's patch, fix it?
>
> cheers
>
> diff --git a/arch/powerpc/mm/hash_utils_64.c b/arch/powerpc/mm/hash_utils_64.c
> index db556d2..1ade7eb 100644
> --- a/arch/powerpc/mm/hash_utils_64.c
> +++ b/arch/powerpc/mm/hash_utils_64.c
> @@ -753,7 +753,7 @@ void __init early_init_mmu(void)
>  }
>
>  #ifdef CONFIG_SMP
> -void __init early_init_mmu_secondary(void)
> +void __cpuinit early_init_mmu_secondary(void)
>  {
>         /* Initialize hash table for that CPU */
>         if (!firmware_has_feature(FW_FEATURE_LPAR))
Yes, this patch fixed the issue. Now i can offline/online cpus without 
any problem.

Thanks
-Sachin

-- 

---------------------------------
Sachin Sant
IBM Linux Technology Center
India Systems and Technology Labs
Bangalore, India
---------------------------------

^ permalink raw reply

* Re: Looking for good supported South bridge for mpc6841d
From: Zhivko Yordanov @ 2009-04-16  9:20 UTC (permalink / raw)
  To: Roderick Colenbrander, Linuxppc-dev
In-Reply-To: <c8485d470904150517p2ed7f27dn9cfd589eb6b7a0a6@mail.gmail.com>

On Wednesday 15 April 2009 14:17:47 you wrote:
> On Wed, Apr 15, 2009 at 1:23 PM, Zhivko Yordanov <jivkojj@uni-kassel.de>=
=20
wrote:
> > Hello,
> >
> > I'm on designing of a new embedded board, based on mpc6841d and South
> > Bridge ULi M1575. As =D0=B0 reference design, I use Freescale HPCN boar=
d.
> > However after consult with chip's distributor, it became clear that the
> > M1575 is already discounted. Now I'm in looking of an another solution.
> > Can somebody recommend me an alternative, good supported from u-boot and
> > have SATA, USB, Audio on it? The chip must be connected with mpc6841d
> > over PCI-Express.
> >
> > Best regarsds,
> >
> > Yordanov
> > _______________________________________________
> > Linuxppc-dev mailing list
> > Linuxppc-dev@ozlabs.org
> > https://ozlabs.org/mailman/listinfo/linuxppc-dev
>
> I think the only alternatives you have are chips from Via and perhaps
> Sis. It looks like Via has some nice south bridges which do what you
> want, see:
> http://www.via.com.tw/en/products/chipsets/south-bridges.jsp
>
> Roderick

Thank you for the anser!

Unfortunately VIA and SIS uses own connection bus between North and Sout=20
bridges - V-map and MuTiol.
I found AMD's SB600 and SB7x0 bridges, that seems to connect through=20
PCI-Express. Are they from u-boot spupported?

Best regards

^ permalink raw reply

* Re: OF PCI howto?
From: Roderick Colenbrander @ 2009-04-16  9:03 UTC (permalink / raw)
  To: Joakim Tjernlund; +Cc: linuxppc-dev
In-Reply-To: <OF08A8F393.64CD0E94-ONC125759A.00276611-C125759A.002C76B2@transmode.se>

On Thu, Apr 16, 2009 at 10:05 AM, Joakim Tjernlund
<Joakim.Tjernlund@transmode.se> wrote:
> Kumar Gala <galak@kernel.crashing.org> wrote on 15/04/2009 17:20:21:
>> On Apr 15, 2009, at 8:08 AM, Wolfram Sang wrote:
>>
>> > On Wed, Apr 15, 2009 at 02:54:57PM +0200, Joakim Tjernlund wrote:
>> >
>> >> dts fragment correct for my setup? If not, is there a better
>> >> example I can
>> >> look at?
>> >
>> > Maybe this message/thread can help you:
>> >
>> > http://ozlabs.org/pipermail/devicetree-discuss/2009-March/000597.html
>>
>> You may also want to take a look at ePAPR on the power.org site
>
> Thanks guys
>
> Looking in ePAPR I see I should read the IDSEL like this:
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0/* IDSEL 0x11 AD17 */
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A00x8800 0x0 0x0 0x1 &ipic 20 0x8 /* INTA */
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A00x8800 0x0 0x0 0x2 &ipic 21 0x8 /* INTB */
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A00x8800 0x0 0x0 0x3 &ipic 22 0x8 /* INTC */
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A00x8800 0x0 0x0 0x4 &ipic 23 0x8 /* INTD */
>
> grouping the first line into:
> "0x8800 0x0 0x0" "0x1" "&ipic" "20 0x8" /* INTA */
>
> "0x8800 0x0 0x0" =3D child unit address
> "0x1" =3D child interrupt specifier =3D INTA
> "&ipic" =3D interrupt parent
> "20 0x8" =3D parent interrupt specifier
>
> Questions:
> =A0MPC832x appears to only have INTA so why are the INTB,INTC
> =A0and INTD lines there?
>
> =A0I don't understand how IDSEL values are decided. Are IDSEL 0x11-0x18
> randomly
> =A0selected or are they fixed for MPC832x?

The idsel values depend on how the pci slots are wired on your board.
The 0x8800 value is
in general a function of bus, device and function number. ( bus << 16
| devfn << 8)

Further I would also watch out with the parent interrupt specifier
e.g. '20 0x8'. The openfirmware spec
normally suggests to use a value of '2' for interrupts which are
active low. During parsing of the interrupt-map
this value is normally translated using 'xlate' to linux kernel values
(active low is defined as IRQ_TYPE_LEVEL_LOW      0x00000008 in the
kernel).
The ipic driver code and various dts files are evil and store the
linux kernel value inside the DTS file, so they don't do any
conversion whis is EVIL
in my opinion. Watch out for that. (The corresponding DTS files and
drivers should be updated).

Regards,
Roderick Colenbrander

^ permalink raw reply

* Question about windfarm drivers
From: Jean Delvare @ 2009-04-16  8:52 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras; +Cc: linuxppc-dev

Hi Ben, hi Paul,

As I am converting the windfarm drivers to the new i2c device binding
model, I need to understand how these drivers work currently. There's
one thing I do not understand so I'd appreciate if you could explain it
to me.

I am looking at function wf_lm75_detach() in windfarm_lm75_sensor.c.
This function is called by i2c-core if either i2c-powermac or
windfarm_lm75_sensor is unloaded. It sets lm->i2c.adapter to NULL and
then calls wf_unregister_sensor(), which in turn calls wf_put_sensor(),
which calls wf_sensor_release() is the reference count drops to 0,
which in turns call the driver's .release() method, that is,
wf_lm75_release() in our case.

In wf_lm75_release(), i2c_detach_client() is called if and only if
lm->i2c.adapter is set, which is not the case, and then the data
structure, including the i2c client, is freed from memory. This means
that the freed i2c client is still registered with i2c-core, this looks
wrong.

Am I missing something? Or is this clean-up path broken and nobody ever
noticed?

I am also curious why wf_unregister_sensor() calls wf_put_sensor()
while wf_register_sensor() doesn't call wf_get_sensor().

Thanks,
-- 
Jean Delvare

^ permalink raw reply

* Re: [ppc64] 2.6.29-git7 : offlining a cpu causes an exception
From: Michael Ellerman @ 2009-04-16  8:25 UTC (permalink / raw)
  To: Sachin Sant; +Cc: linuxppc-dev
In-Reply-To: <49E6C3F4.4080801@in.ibm.com>

On Thu, 2009-04-16 at 11:06 +0530, Sachin Sant wrote:
> Sachin Sant wrote:
> > Sachin Sant wrote:
> >> Benjamin Herrenschmidt wrote:
> >>> On Tue, 2009-03-31 at 14:57 +0530, Sachin Sant wrote:
> >>>  
> >>>> While executing CPU HotPlug[1] tests i observed that during
> >>>> every cpu offline process an exception is thrown.
> >>>>     
> >>>
> >>> Looks like a BUG_ON() to me... can you look at what other
> >>> messages just before that ?  
> >>
> > Ben, seems like the following patch is causing the cpu hotplug
> > test failure.
> > [PATCH 6/6] powerpc/mm: Introduce early_init_mmu() on 64-bit
> >
> > http://ozlabs.org/pipermail/linuxppc-dev/2009-March/069613.html
> >
> > If i back out this patch, i am able to offline/online cpu's
> > without any issue.
> I can recreate this problem with 2.6.30-rc2-git1 as well. Same BUG_ON while
> running cpu hotplug tests.
> 
> Let me know if there is any thing i can help to find a fix for this.

Hi Sachin,

Does this patch, on top of Ben's patch, fix it?

cheers

diff --git a/arch/powerpc/mm/hash_utils_64.c b/arch/powerpc/mm/hash_utils_64.c
index db556d2..1ade7eb 100644
--- a/arch/powerpc/mm/hash_utils_64.c
+++ b/arch/powerpc/mm/hash_utils_64.c
@@ -753,7 +753,7 @@ void __init early_init_mmu(void)
 }
 
 #ifdef CONFIG_SMP
-void __init early_init_mmu_secondary(void)
+void __cpuinit early_init_mmu_secondary(void)
 {
        /* Initialize hash table for that CPU */
        if (!firmware_has_feature(FW_FEATURE_LPAR))

^ permalink raw reply related

* Re: OF PCI howto?
From: Joakim Tjernlund @ 2009-04-16  8:05 UTC (permalink / raw)
  To: Kumar Gala; +Cc: linuxppc-dev
In-Reply-To: <8BE09C31-40E4-41C2-A6D8-EE9C7DA0B625@kernel.crashing.org>

Kumar Gala <galak@kernel.crashing.org> wrote on 15/04/2009 17:20:21:
> On Apr 15, 2009, at 8:08 AM, Wolfram Sang wrote:
> 
> > On Wed, Apr 15, 2009 at 02:54:57PM +0200, Joakim Tjernlund wrote:
> >
> >> dts fragment correct for my setup? If not, is there a better 
> >> example I can
> >> look at?
> >
> > Maybe this message/thread can help you:
> >
> > http://ozlabs.org/pipermail/devicetree-discuss/2009-March/000597.html
> 
> You may also want to take a look at ePAPR on the power.org site

Thanks guys

Looking in ePAPR I see I should read the IDSEL like this:
                /* IDSEL 0x11 AD17 */
                0x8800 0x0 0x0 0x1 &ipic 20 0x8 /* INTA */
                0x8800 0x0 0x0 0x2 &ipic 21 0x8 /* INTB */
                0x8800 0x0 0x0 0x3 &ipic 22 0x8 /* INTC */
                0x8800 0x0 0x0 0x4 &ipic 23 0x8 /* INTD */

grouping the first line into:
"0x8800 0x0 0x0" "0x1" "&ipic" "20 0x8" /* INTA */

"0x8800 0x0 0x0" = child unit address
"0x1" = child interrupt specifier = INTA
"&ipic" = interrupt parent
"20 0x8" = parent interrupt specifier

Questions:
 MPC832x appears to only have INTA so why are the INTB,INTC 
 and INTD lines there?

 I don't understand how IDSEL values are decided. Are IDSEL 0x11-0x18 
randomly
 selected or are they fixed for MPC832x?

What I really trying to understand is what fields I might need to change
and what to change them to.
So far the "interrupt-map" and the "ranges" items looks like they might 
need
some tweaking, but I really don't know at this point.
Any pointers in this area is much appreciated.

 Jocke

^ permalink raw reply

* Re: [PATCH] AOA: Convert onyx and tas codecs to new-style i2c drivers
From: Takashi Iwai @ 2009-04-16  7:56 UTC (permalink / raw)
  To: Jean Delvare; +Cc: linuxppc-dev, Johannes Berg, alsa-devel
In-Reply-To: <20090416095339.5cd026a0@hyperion.delvare>

At Thu, 16 Apr 2009 09:53:39 +0200,
Jean Delvare wrote:
> 
> On Tue, 14 Apr 2009 16:45:38 +0200, Takashi Iwai wrote:
> > Johannes, please let me know if the patch works.  Then I'll merge them.
> 
> Note if it matters: the new I2C binding model my patch uses is only
> available since kernel 2.6.26.

Yep, I'll add some backward compatible stuff to alsa-driver external
tree so that it can be built for older kernels.


thanks,

Takashi

^ permalink raw reply

* Re: [PATCH] AOA: Convert onyx and tas codecs to new-style i2c   drivers
From: Jean Delvare @ 2009-04-16  7:53 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: linuxppc-dev, Johannes Berg, alsa-devel
In-Reply-To: <s5h1vrv2t0t.wl%tiwai@suse.de>

On Tue, 14 Apr 2009 16:45:38 +0200, Takashi Iwai wrote:
> Johannes, please let me know if the patch works.  Then I'll merge them.

Note if it matters: the new I2C binding model my patch uses is only
available since kernel 2.6.26.

-- 
Jean Delvare

^ permalink raw reply

* [v3 PATCH 3/3] EDAC: CPC925 MC platform device setup
From: Harry Ciao @ 2009-04-16  5:51 UTC (permalink / raw)
  To: akpm, michael, galak, bluesmoke-devel, linuxppc-dev; +Cc: linux-kernel


Hi Andrew and Michael,

This is the modified 3/3 patch that corrects CPC925 memory controller DTB node on Maple and setup related platform device for CPC925 EDAC driver. Michael's suggestion of being more discernible for the Maple platform only has been adopted.

v2 1/3 & 2/3 patches have been in -mm tree and I don't have to re-send them again. Test passed for this v3 3/3 patch.

Thanks!

Best regards,

Harry

^ permalink raw reply

* [v3 PATCH 3/3] EDAC: CPC925 MC platform device setup
From: Harry Ciao @ 2009-04-16  5:51 UTC (permalink / raw)
  To: akpm, michael, galak, bluesmoke-devel, linuxppc-dev
  Cc: Harry Ciao, linux-kernel
In-Reply-To: <1239861063-22837-1-git-send-email-qingtao.cao@windriver.com>

Fixup the number of cells for the values of CPC925 Memory Controller,
and setup related platform device during system booting up, against 
which CPC925 Memory Controller EDAC driver would be matched.

Signed-off-by: Harry Ciao <qingtao.cao@windriver.com>
---
 arch/powerpc/kernel/prom_init.c      |   40 +++++++++++++++++++++++
 arch/powerpc/platforms/maple/setup.c |   59 +++++++++++++++++++++++++++++++++++
 2 files changed, 99 insertions(+)

--- a/arch/powerpc/platforms/maple/setup.c
+++ b/arch/powerpc/platforms/maple/setup.c
@@ -335,3 +335,62 @@ define_machine(maple) {
 	.progress		= maple_progress,
 	.power_save		= power4_idle,
 };
+
+#ifdef CONFIG_EDAC
+/*
+ * Register a platform device for CPC925 memory controller on
+ * Motorola ATCA-6101 blade.
+ */
+#define MAPLE_CPC925_MODEL	"Motorola,ATCA-6101"
+static int __init maple_cpc925_edac_setup(void)
+{
+	struct platform_device *pdev;
+	struct device_node *np = NULL;
+	struct resource r;
+	const unsigned char *model;
+	int ret;
+
+	np = of_find_node_by_path("/");
+	if (!np) {
+		printk(KERN_ERR "%s: Unable to get root node\n", __func__);
+		return -ENODEV;
+	}
+
+	model = (const unsigned char *)of_get_property(np, "model", NULL);
+	if (!model) {
+		printk(KERN_ERR "%s: Unabel to get model info\n", __func__);
+		return -ENODEV;
+	}
+
+	ret = strcmp(model, MAPLE_CPC925_MODEL);
+	of_node_put(np);
+
+	if (ret != 0)
+		return 0;
+
+	np = of_find_node_by_type(NULL, "memory-controller");
+	if (!np) {
+		printk(KERN_ERR "%s: Unable to find memory-controller node\n",
+			__func__);
+		return -ENODEV;
+	}
+
+	ret = of_address_to_resource(np, 0, &r);
+	of_node_put(np);
+
+	if (ret < 0) {
+		printk(KERN_ERR "%s: Unable to get memory-controller reg\n",
+			__func__);
+		return -ENODEV;
+	}
+
+	pdev = platform_device_register_simple("cpc925_edac", 0, &r, 1);
+	if (IS_ERR(pdev))
+		return PTR_ERR(pdev);
+
+	printk(KERN_INFO "%s: CPC925 platform device created\n", __func__);
+
+	return 0;
+}
+machine_device_initcall(maple, maple_cpc925_edac_setup);
+#endif
--- a/arch/powerpc/kernel/prom_init.c
+++ b/arch/powerpc/kernel/prom_init.c
@@ -1948,8 +1948,47 @@ static void __init fixup_device_tree_map
 	prom_setprop(isa, name, "ranges",
 			isa_ranges, sizeof(isa_ranges));
 }
+
+#define CPC925_MC_START		0xf8000000
+#define CPC925_MC_LENGTH	0x1000000
+/* The values for memory-controller don't have right number of cells */
+static void __init fixup_device_tree_maple_memory_controller(void)
+{
+	phandle mc;
+	u32 mc_reg[4];
+	char *name = "/hostbridge@f8000000";
+	struct prom_t *_prom = &RELOC(prom);
+	u32 ac, sc;
+
+	mc = call_prom("finddevice", 1, 1, ADDR(name));
+	if (!PHANDLE_VALID(mc))
+		return;
+
+	if (prom_getproplen(mc, "reg") != 8)
+		return;
+
+	prom_getprop(_prom->root, "#address-cells", &ac, sizeof(ac));
+	prom_getprop(_prom->root, "#size-cells", &sc, sizeof(sc));
+	if ((ac != 2) || (sc != 2))
+		return;
+
+	if (prom_getprop(mc, "reg", mc_reg, sizeof(mc_reg)) == PROM_ERROR)
+		return;
+
+	if (mc_reg[0] != CPC925_MC_START || mc_reg[1] != CPC925_MC_LENGTH)
+		return;
+
+	prom_printf("Fixing up bogus hostbridge on Maple...\n");
+
+	mc_reg[0] = 0x0;
+	mc_reg[1] = CPC925_MC_START;
+	mc_reg[2] = 0x0;
+	mc_reg[3] = CPC925_MC_LENGTH;
+	prom_setprop(mc, name, "reg", mc_reg, sizeof(mc_reg));
+}
 #else
 #define fixup_device_tree_maple()
+#define fixup_device_tree_maple_memory_controller()
 #endif
 
 #ifdef CONFIG_PPC_CHRP
@@ -2190,6 +2229,7 @@ static void __init fixup_device_tree_efi
 static void __init fixup_device_tree(void)
 {
 	fixup_device_tree_maple();
+	fixup_device_tree_maple_memory_controller();
 	fixup_device_tree_chrp();
 	fixup_device_tree_pmac();
 	fixup_device_tree_efika();

^ permalink raw reply

* Re: [ppc64] 2.6.29-git7 : offlining a cpu causes an exception
From: Sachin Sant @ 2009-04-16  5:36 UTC (permalink / raw)
  To: linuxppc-dev
In-Reply-To: <49D3547E.8030200@in.ibm.com>

Sachin Sant wrote:
> Sachin Sant wrote:
>> Benjamin Herrenschmidt wrote:
>>> On Tue, 2009-03-31 at 14:57 +0530, Sachin Sant wrote:
>>>  
>>>> While executing CPU HotPlug[1] tests i observed that during
>>>> every cpu offline process an exception is thrown.
>>>>     
>>>
>>> Looks like a BUG_ON() to me... can you look at what other
>>> messages just before that ?  
>>
> Ben, seems like the following patch is causing the cpu hotplug
> test failure.
> [PATCH 6/6] powerpc/mm: Introduce early_init_mmu() on 64-bit
>
> http://ozlabs.org/pipermail/linuxppc-dev/2009-March/069613.html
>
> If i back out this patch, i am able to offline/online cpu's
> without any issue.
I can recreate this problem with 2.6.30-rc2-git1 as well. Same BUG_ON while
running cpu hotplug tests.

Let me know if there is any thing i can help to find a fix for this.

Thanks
-Sachin


-- 

---------------------------------
Sachin Sant
IBM Linux Technology Center
India Systems and Technology Labs
Bangalore, India
---------------------------------

^ permalink raw reply

* Re: + edac-cpc925-mc-platform-device-setup.patch added to -mm tree
From: Harry Ciao @ 2009-04-16  3:40 UTC (permalink / raw)
  To: michael
  Cc: mm-commits, Linuxppc-dev Development, Paul Mackerras, Kumar Gala,
	Doug Thompson, akpm
In-Reply-To: <1239848054.6572.20.camel@localhost>

Michael Ellerman wrote:
> On Thu, 2009-04-16 at 09:57 +0800, Harry Ciao wrote:
>   
>> Kumar Gala wrote:
>>     
>>> On Apr 15, 2009, at 5:27 PM, akpm@linux-foundation.org wrote:
>>>       
>>>> arch/powerpc/kernel/prom_init.c      |   33 +++++++++++++
>>>> arch/powerpc/platforms/maple/setup.c |   59 +++++++++++++++++++++++++
>>>> 2 files changed, 92 insertions(+)
>>>>
>>>> diff -puN 
>>>> arch/powerpc/kernel/prom_init.c~edac-cpc925-mc-platform-device-setup 
>>>> arch/powerpc/kernel/prom_init.c
>>>> --- 
>>>> a/arch/powerpc/kernel/prom_init.c~edac-cpc925-mc-platform-device-setup
>>>> +++ a/arch/powerpc/kernel/prom_init.c
>>>> @@ -1947,8 +1947,40 @@ static void __init fixup_device_tree_map
>>>>     prom_setprop(isa, name, "ranges",
>>>>             isa_ranges, sizeof(isa_ranges));
>>>> }
>>>> +
>>>> +#define CPC925_MC_START        0xf8000000
>>>> +#define CPC925_MC_LENGTH    0x1000000
>>>> +/* The values for memory-controller don't have right number of cells */
>>>> +static void __init fixup_device_tree_maple_memory_controller(void)
>>>> +{
>>>>         
>>> I don't see why this cant be part of the existing 
>>> fixup_device_tree_maple().
>>>
>>> I also find it odd we don't ensure we are running on a maple before we 
>>> apply this fixup.
>>>       
>
>   
>> Hi Kumar,
>>
>> Thanks a lot for your concern.
>>
>> This newly added fixup for memory controller on Maple will be placed 
>> right after fixup_device_tree_maple(), both of them will be controlled 
>> by CONFIG_PPC_MAPLE, so there is no worry that it will be applied 
>> against anything other than Maple.
>>     
>
> Hi Harry,
>
> We regularly build a single kernel with multiple platforms enabled, so
> just having it controlled by a CONFIG symbol is not sufficient. Someone
> might build a kernel for MAPLE & PSERIES & ISERIES & CELL, so the maple
> fixup needs to be careful it doesn't break the other platforms.
>
> The existing maple fixup doesn't check if it's on a maple either, but it
> is a bit more discerning about what it finds before it fixes things up.
>
> Your code already checks that "reg" is 8 bytes long to start with, I
> think if it also checks that #address-cells and #size-cells are == 2,
> then it's pretty safe. Because at that point we know we have a node with
> the right name, the reg property has a known value, and reg is short WRT
> #cells.
>
>   
Many thanks Michael!

That makes a lot of sense to me :) I will integrate the check if both 
its parent #address-cells and #size-cells equal to 2 before fixing 
anything up. The fact that the reg length == 8 bytes whereas parent's 
cells are 2 validate our fixup is necessary.

Best regards,

Harry


>> Meanwhile, it aims at fixup bad cell numbers for the memory controller, 
>> whereas the original fixup_device_tree_maple() aiming at fixing up the 
>> ISA controller on HT channel, we'd better separate them in different 
>> function IMHO.
>>     
>
> I think I agree it's better as a separate routine. We could have a
> firmware that doesn't need the original maple fixup (and so exits from
> that routine early) but does need this one.
>
> cheers
>
>   

^ permalink raw reply

* Re: + edac-cpc925-mc-platform-device-setup.patch added to -mm tree
From: Michael Ellerman @ 2009-04-16  2:14 UTC (permalink / raw)
  To: qingtao.cao
  Cc: mm-commits, Linuxppc-dev Development, Paul Mackerras, Kumar Gala,
	Doug Thompson, akpm
In-Reply-To: <49E69098.6040903@windriver.com>

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

On Thu, 2009-04-16 at 09:57 +0800, Harry Ciao wrote:
> Kumar Gala wrote:
> > On Apr 15, 2009, at 5:27 PM, akpm@linux-foundation.org wrote:
> >> arch/powerpc/kernel/prom_init.c      |   33 +++++++++++++
> >> arch/powerpc/platforms/maple/setup.c |   59 +++++++++++++++++++++++++
> >> 2 files changed, 92 insertions(+)
> >>
> >> diff -puN 
> >> arch/powerpc/kernel/prom_init.c~edac-cpc925-mc-platform-device-setup 
> >> arch/powerpc/kernel/prom_init.c
> >> --- 
> >> a/arch/powerpc/kernel/prom_init.c~edac-cpc925-mc-platform-device-setup
> >> +++ a/arch/powerpc/kernel/prom_init.c
> >> @@ -1947,8 +1947,40 @@ static void __init fixup_device_tree_map
> >>     prom_setprop(isa, name, "ranges",
> >>             isa_ranges, sizeof(isa_ranges));
> >> }
> >> +
> >> +#define CPC925_MC_START        0xf8000000
> >> +#define CPC925_MC_LENGTH    0x1000000
> >> +/* The values for memory-controller don't have right number of cells */
> >> +static void __init fixup_device_tree_maple_memory_controller(void)
> >> +{
> >
> > I don't see why this cant be part of the existing 
> > fixup_device_tree_maple().
> >
> > I also find it odd we don't ensure we are running on a maple before we 
> > apply this fixup.

> Hi Kumar,
> 
> Thanks a lot for your concern.
> 
> This newly added fixup for memory controller on Maple will be placed 
> right after fixup_device_tree_maple(), both of them will be controlled 
> by CONFIG_PPC_MAPLE, so there is no worry that it will be applied 
> against anything other than Maple.

Hi Harry,

We regularly build a single kernel with multiple platforms enabled, so
just having it controlled by a CONFIG symbol is not sufficient. Someone
might build a kernel for MAPLE & PSERIES & ISERIES & CELL, so the maple
fixup needs to be careful it doesn't break the other platforms.

The existing maple fixup doesn't check if it's on a maple either, but it
is a bit more discerning about what it finds before it fixes things up.

Your code already checks that "reg" is 8 bytes long to start with, I
think if it also checks that #address-cells and #size-cells are == 2,
then it's pretty safe. Because at that point we know we have a node with
the right name, the reg property has a known value, and reg is short WRT
#cells.

> Meanwhile, it aims at fixup bad cell numbers for the memory controller, 
> whereas the original fixup_device_tree_maple() aiming at fixing up the 
> ISA controller on HT channel, we'd better separate them in different 
> function IMHO.

I think I agree it's better as a separate routine. We could have a
firmware that doesn't need the original maple fixup (and so exits from
that routine early) but does need this one.

cheers

-- 
Michael Ellerman
OzLabs, IBM Australia Development Lab

wwweb: http://michael.ellerman.id.au
phone: +61 2 6212 1183 (tie line 70 21183)

We do not inherit the earth from our ancestors,
we borrow it from our children. - S.M.A.R.T Person

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply

* Re: + edac-cpc925-mc-platform-device-setup.patch added to -mm tree
From: Harry Ciao @ 2009-04-16  1:57 UTC (permalink / raw)
  To: Kumar Gala
  Cc: mm-commits, Linuxppc-dev Development, Paul Mackerras, Kumar Gala,
	Doug Thompson, akpm
In-Reply-To: <98F905BD-B447-4FAA-A7FE-9FCD025A9079@kernel.crashing.org>

Kumar Gala wrote:
>
> On Apr 15, 2009, at 5:27 PM, akpm@linux-foundation.org wrote:
>
>>
>> The patch titled
>>     edac: cpc925 MC platform device setup
>> has been added to the -mm tree.  Its filename is
>>     edac-cpc925-mc-platform-device-setup.patch
>>
>> Before you just go and hit "reply", please:
>>   a) Consider who else should be cc'ed
>>   b) Prefer to cc a suitable mailing list as well
>>   c) Ideally: find the original patch on the mailing list and do a
>>      reply-to-all to that, adding suitable additional cc's
>>
>> *** Remember to use Documentation/SubmitChecklist when testing your 
>> code ***
>>
>> See http://userweb.kernel.org/~akpm/stuff/added-to-mm.txt to find
>> out what to do about this
>>
>> The current -mm tree may be found at 
>> http://userweb.kernel.org/~akpm/mmotm/
>>
>> ------------------------------------------------------
>> Subject: edac: cpc925 MC platform device setup
>> From: Harry Ciao <qingtao.cao@windriver.com>
>>
>> Fix up the number of cells for the values of CPC925 Memory Controller,
>> and setup related platform device during system booting up, against
>> which CPC925 Memory Controller EDAC driver would be matched.
>>
>> Signed-off-by: Harry Ciao <qingtao.cao@windriver.com>
>> Cc: Doug Thompson <norsk5@yahoo.com>
>> Cc: Michael Ellerman <michael@ellerman.id.au>
>> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>> Cc: Kumar Gala <galak@gate.crashing.org>
>> Cc: Paul Mackerras <paulus@samba.org>
>> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
>> ---
>>
>> arch/powerpc/kernel/prom_init.c      |   33 +++++++++++++
>> arch/powerpc/platforms/maple/setup.c |   59 +++++++++++++++++++++++++
>> 2 files changed, 92 insertions(+)
>>
>> diff -puN 
>> arch/powerpc/kernel/prom_init.c~edac-cpc925-mc-platform-device-setup 
>> arch/powerpc/kernel/prom_init.c
>> --- 
>> a/arch/powerpc/kernel/prom_init.c~edac-cpc925-mc-platform-device-setup
>> +++ a/arch/powerpc/kernel/prom_init.c
>> @@ -1947,8 +1947,40 @@ static void __init fixup_device_tree_map
>>     prom_setprop(isa, name, "ranges",
>>             isa_ranges, sizeof(isa_ranges));
>> }
>> +
>> +#define CPC925_MC_START        0xf8000000
>> +#define CPC925_MC_LENGTH    0x1000000
>> +/* The values for memory-controller don't have right number of cells */
>> +static void __init fixup_device_tree_maple_memory_controller(void)
>> +{
>
> I don't see why this cant be part of the existing 
> fixup_device_tree_maple().
>
> I also find it odd we don't ensure we are running on a maple before we 
> apply this fixup.
>
> - k
>
Hi Kumar,

Thanks a lot for your concern.

This newly added fixup for memory controller on Maple will be placed 
right after fixup_device_tree_maple(), both of them will be controlled 
by CONFIG_PPC_MAPLE, so there is no worry that it will be applied 
against anything other than Maple.

Meanwhile, it aims at fixup bad cell numbers for the memory controller, 
whereas the original fixup_device_tree_maple() aiming at fixing up the 
ISA controller on HT channel, we'd better separate them in different 
function IMHO.

Best regards,

Harry


>>
>> +    phandle mc;
>> +    u32 mc_reg[4];
>> +    char *name = "/hostbridge@f8000000";
>> +
>> +    mc = call_prom("finddevice", 1, 1, ADDR(name));
>> +    if (!PHANDLE_VALID(mc))
>> +        return;
>> +
>> +    if (prom_getproplen(mc, "reg") != 8)
>> +        return;
>> +
>> +    if (prom_getprop(mc, "reg", mc_reg, sizeof(mc_reg)) == PROM_ERROR)
>> +        return;
>> +
>> +    if (mc_reg[0] != CPC925_MC_START || mc_reg[1] != CPC925_MC_LENGTH)
>> +        return;
>> +
>> +    prom_printf("Fixing up bogus hostbridge on Maple...\n");
>> +
>> +    mc_reg[0] = 0x0;
>> +    mc_reg[1] = CPC925_MC_START;
>> +    mc_reg[2] = 0x0;
>> +    mc_reg[3] = CPC925_MC_LENGTH;
>> +    prom_setprop(mc, name, "reg", mc_reg, sizeof(mc_reg));
>> +}
>> #else
>> #define fixup_device_tree_maple()
>> +#define fixup_device_tree_maple_memory_controller()
>> #endif
>>
>> #ifdef CONFIG_PPC_CHRP
>> @@ -2189,6 +2221,7 @@ static void __init fixup_device_tree_efi
>> static void __init fixup_device_tree(void)
>> {
>>     fixup_device_tree_maple();
>> +    fixup_device_tree_maple_memory_controller();
>>     fixup_device_tree_chrp();
>>     fixup_device_tree_pmac();
>>     fixup_device_tree_efika();
>> diff -puN 
>> arch/powerpc/platforms/maple/setup.c~edac-cpc925-mc-platform-device-setup 
>> arch/powerpc/platforms/maple/setup.c
>> --- 
>> a/arch/powerpc/platforms/maple/setup.c~edac-cpc925-mc-platform-device-setup 
>>
>> +++ a/arch/powerpc/platforms/maple/setup.c
>> @@ -335,3 +335,62 @@ define_machine(maple) {
>>     .progress        = maple_progress,
>>     .power_save        = power4_idle,
>> };
>> +
>> +#ifdef CONFIG_EDAC
>> +/*
>> + * Register a platform device for CPC925 memory controller on
>> + * Motorola ATCA-6101 blade.
>> + */
>> +#define MAPLE_CPC925_MODEL    "Motorola,ATCA-6101"
>> +static int __init maple_cpc925_edac_setup(void)
>> +{
>> +    struct platform_device *pdev;
>> +    struct device_node *np = NULL;
>> +    struct resource r;
>> +    const unsigned char *model;
>> +    int ret;
>> +
>> +    np = of_find_node_by_path("/");
>> +    if (!np) {
>> +        printk(KERN_ERR "%s: Unable to get root node\n", __func__);
>> +        return -ENODEV;
>> +    }
>> +
>> +    model = (const unsigned char *)of_get_property(np, "model", NULL);
>> +    if (!model) {
>> +        printk(KERN_ERR "%s: Unabel to get model info\n", __func__);
>> +        return -ENODEV;
>> +    }
>> +
>> +    ret = strcmp(model, MAPLE_CPC925_MODEL);
>> +    of_node_put(np);
>> +
>> +    if (ret != 0)
>> +        return 0;
>> +
>> +    np = of_find_node_by_type(NULL, "memory-controller");
>> +    if (!np) {
>> +        printk(KERN_ERR "%s: Unable to find memory-controller node\n",
>> +            __func__);
>> +        return -ENODEV;
>> +    }
>> +
>> +    ret = of_address_to_resource(np, 0, &r);
>> +    of_node_put(np);
>> +
>> +    if (ret < 0) {
>> +        printk(KERN_ERR "%s: Unable to get memory-controller reg\n",
>> +            __func__);
>> +        return -ENODEV;
>> +    }
>> +
>> +    pdev = platform_device_register_simple("cpc925_edac", 0, &r, 1);
>> +    if (IS_ERR(pdev))
>> +        return PTR_ERR(pdev);
>> +
>> +    printk(KERN_INFO "%s: CPC925 platform device created\n", __func__);
>> +
>> +    return 0;
>> +}
>> +machine_device_initcall(maple, maple_cpc925_edac_setup);
>> +#endif
>> _
>>
>> Patches currently in -mm which might be from 
>> qingtao.cao@windriver.com are
>>
>> edac-add-cpc925-memory-controller-driver.patch
>> edac-add-cpc925-memory-controller-driver-cleanup.patch
>> edac-add-edac_device_alloc_index.patch
>> edac-add-edac_device_alloc_index-cleanup.patch
>> edac-cpc925-mc-platform-device-setup.patch
>

^ permalink raw reply

* Re: + edac-cpc925-mc-platform-device-setup.patch added to -mm tree
From: Kumar Gala @ 2009-04-15 23:58 UTC (permalink / raw)
  To: akpm
  Cc: mm-commits, Linuxppc-dev Development, Paul Mackerras, Kumar Gala,
	qingtao.cao, Doug Thompson
In-Reply-To: <200904152227.n3FMRmdm007192@imap1.linux-foundation.org>


On Apr 15, 2009, at 5:27 PM, akpm@linux-foundation.org wrote:

>
> The patch titled
>     edac: cpc925 MC platform device setup
> has been added to the -mm tree.  Its filename is
>     edac-cpc925-mc-platform-device-setup.patch
>
> Before you just go and hit "reply", please:
>   a) Consider who else should be cc'ed
>   b) Prefer to cc a suitable mailing list as well
>   c) Ideally: find the original patch on the mailing list and do a
>      reply-to-all to that, adding suitable additional cc's
>
> *** Remember to use Documentation/SubmitChecklist when testing your  
> code ***
>
> See http://userweb.kernel.org/~akpm/stuff/added-to-mm.txt to find
> out what to do about this
>
> The current -mm tree may be found at http://userweb.kernel.org/~akpm/mmotm/
>
> ------------------------------------------------------
> Subject: edac: cpc925 MC platform device setup
> From: Harry Ciao <qingtao.cao@windriver.com>
>
> Fix up the number of cells for the values of CPC925 Memory Controller,
> and setup related platform device during system booting up, against
> which CPC925 Memory Controller EDAC driver would be matched.
>
> Signed-off-by: Harry Ciao <qingtao.cao@windriver.com>
> Cc: Doug Thompson <norsk5@yahoo.com>
> Cc: Michael Ellerman <michael@ellerman.id.au>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Kumar Gala <galak@gate.crashing.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> ---
>
> arch/powerpc/kernel/prom_init.c      |   33 +++++++++++++
> arch/powerpc/platforms/maple/setup.c |   59 +++++++++++++++++++++++++
> 2 files changed, 92 insertions(+)
>
> diff -puN arch/powerpc/kernel/prom_init.c~edac-cpc925-mc-platform- 
> device-setup arch/powerpc/kernel/prom_init.c
> --- a/arch/powerpc/kernel/prom_init.c~edac-cpc925-mc-platform-device- 
> setup
> +++ a/arch/powerpc/kernel/prom_init.c
> @@ -1947,8 +1947,40 @@ static void __init fixup_device_tree_map
> 	prom_setprop(isa, name, "ranges",
> 			isa_ranges, sizeof(isa_ranges));
> }
> +
> +#define CPC925_MC_START		0xf8000000
> +#define CPC925_MC_LENGTH	0x1000000
> +/* The values for memory-controller don't have right number of  
> cells */
> +static void __init fixup_device_tree_maple_memory_controller(void)
> +{

I don't see why this cant be part of the existing  
fixup_device_tree_maple().

I also find it odd we don't ensure we are running on a maple before we  
apply this fixup.

- k

>
> +	phandle mc;
> +	u32 mc_reg[4];
> +	char *name = "/hostbridge@f8000000";
> +
> +	mc = call_prom("finddevice", 1, 1, ADDR(name));
> +	if (!PHANDLE_VALID(mc))
> +		return;
> +
> +	if (prom_getproplen(mc, "reg") != 8)
> +		return;
> +
> +	if (prom_getprop(mc, "reg", mc_reg, sizeof(mc_reg)) == PROM_ERROR)
> +		return;
> +
> +	if (mc_reg[0] != CPC925_MC_START || mc_reg[1] != CPC925_MC_LENGTH)
> +		return;
> +
> +	prom_printf("Fixing up bogus hostbridge on Maple...\n");
> +
> +	mc_reg[0] = 0x0;
> +	mc_reg[1] = CPC925_MC_START;
> +	mc_reg[2] = 0x0;
> +	mc_reg[3] = CPC925_MC_LENGTH;
> +	prom_setprop(mc, name, "reg", mc_reg, sizeof(mc_reg));
> +}
> #else
> #define fixup_device_tree_maple()
> +#define fixup_device_tree_maple_memory_controller()
> #endif
>
> #ifdef CONFIG_PPC_CHRP
> @@ -2189,6 +2221,7 @@ static void __init fixup_device_tree_efi
> static void __init fixup_device_tree(void)
> {
> 	fixup_device_tree_maple();
> +	fixup_device_tree_maple_memory_controller();
> 	fixup_device_tree_chrp();
> 	fixup_device_tree_pmac();
> 	fixup_device_tree_efika();
> diff -puN arch/powerpc/platforms/maple/setup.c~edac-cpc925-mc- 
> platform-device-setup arch/powerpc/platforms/maple/setup.c
> --- a/arch/powerpc/platforms/maple/setup.c~edac-cpc925-mc-platform- 
> device-setup
> +++ a/arch/powerpc/platforms/maple/setup.c
> @@ -335,3 +335,62 @@ define_machine(maple) {
> 	.progress		= maple_progress,
> 	.power_save		= power4_idle,
> };
> +
> +#ifdef CONFIG_EDAC
> +/*
> + * Register a platform device for CPC925 memory controller on
> + * Motorola ATCA-6101 blade.
> + */
> +#define MAPLE_CPC925_MODEL	"Motorola,ATCA-6101"
> +static int __init maple_cpc925_edac_setup(void)
> +{
> +	struct platform_device *pdev;
> +	struct device_node *np = NULL;
> +	struct resource r;
> +	const unsigned char *model;
> +	int ret;
> +
> +	np = of_find_node_by_path("/");
> +	if (!np) {
> +		printk(KERN_ERR "%s: Unable to get root node\n", __func__);
> +		return -ENODEV;
> +	}
> +
> +	model = (const unsigned char *)of_get_property(np, "model", NULL);
> +	if (!model) {
> +		printk(KERN_ERR "%s: Unabel to get model info\n", __func__);
> +		return -ENODEV;
> +	}
> +
> +	ret = strcmp(model, MAPLE_CPC925_MODEL);
> +	of_node_put(np);
> +
> +	if (ret != 0)
> +		return 0;
> +
> +	np = of_find_node_by_type(NULL, "memory-controller");
> +	if (!np) {
> +		printk(KERN_ERR "%s: Unable to find memory-controller node\n",
> +			__func__);
> +		return -ENODEV;
> +	}
> +
> +	ret = of_address_to_resource(np, 0, &r);
> +	of_node_put(np);
> +
> +	if (ret < 0) {
> +		printk(KERN_ERR "%s: Unable to get memory-controller reg\n",
> +			__func__);
> +		return -ENODEV;
> +	}
> +
> +	pdev = platform_device_register_simple("cpc925_edac", 0, &r, 1);
> +	if (IS_ERR(pdev))
> +		return PTR_ERR(pdev);
> +
> +	printk(KERN_INFO "%s: CPC925 platform device created\n", __func__);
> +
> +	return 0;
> +}
> +machine_device_initcall(maple, maple_cpc925_edac_setup);
> +#endif
> _
>
> Patches currently in -mm which might be from  
> qingtao.cao@windriver.com are
>
> edac-add-cpc925-memory-controller-driver.patch
> edac-add-cpc925-memory-controller-driver-cleanup.patch
> edac-add-edac_device_alloc_index.patch
> edac-add-edac_device_alloc_index-cleanup.patch
> edac-cpc925-mc-platform-device-setup.patch

^ permalink raw reply

* Re: [PATCH 00/14] Add common OF device tree support for MDIO busses
From: Andy Fleming @ 2009-04-15 23:06 UTC (permalink / raw)
  To: Grant Likely; +Cc: Joakim Tjernlund, netdev, linuxppc-dev, olof
In-Reply-To: <20090331075537.1427.7819.stgit@localhost.localdomain>

Other than the previous comments, all appropriate patches can be marked:

Acked-by: Andy Fleming <afleming@freescale.com>

^ permalink raw reply

* Re: [PATCH 09/14] net: Rework gianfar driver to use of_mdio infrastructure.
From: Andy Fleming @ 2009-04-15 23:01 UTC (permalink / raw)
  To: Grant Likely; +Cc: Joakim Tjernlund, netdev, linuxppc-dev, olof
In-Reply-To: <20090331082730.1427.96418.stgit@localhost.localdomain>


On Mar 31, 2009, at 3:27 AM, Grant Likely wrote:

> From: Grant Likely <grant.likely@secretlab.ca>
>
> This patch simplifies the driver by making use of more common code.
>
> Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
> ---
>
> drivers/net/gianfar.c |  103 +++++++++++++++++ 
> +-------------------------------
> drivers/net/gianfar.h |    3 +
> 2 files changed, 40 insertions(+), 66 deletions(-)
>
>
> diff --git a/drivers/net/gianfar.c b/drivers/net/gianfar.c
> index 65f5587..c22eba9 100644
> @@ -699,23 +657,38 @@ static int init_phy(struct net_device *dev)
>
>
> +	if (priv->tbi_node) {
> +		priv->tbiphy = of_phy_connect(dev, priv->tbi_node, &adjust_link,
> +					      0, interface);
> +		if (!priv->tbiphy) {
> +			dev_err(&dev->dev, "error: Could not attach to TBI\n");
> +			goto err_tbiphy;
> +		}
> +	}

I don't believe we need this.  Certainly, the current code doesn't do  
anything like this.  The TBI node is a special internal PHY used to  
manage SGMII/TBI links.


Andy

^ permalink raw reply

* Re: [v2 PATCH 2/3] EDAC: Add edac_device_alloc_index()
From: Andrew Morton @ 2009-04-15 22:27 UTC (permalink / raw)
  To: Harry Ciao; +Cc: norsk5, bluesmoke-devel, linux-kernel, linuxppc-dev
In-Reply-To: <1239602716-18352-3-git-send-email-qingtao.cao@windriver.com>

On Mon, 13 Apr 2009 14:05:15 +0800
Harry Ciao <qingtao.cao@windriver.com> wrote:

> Add edac_device_alloc_index(), because for MAPLE platform there may
> exist several EDAC driver modules that could make use of
> edac_device_ctl_info structure at the same time. The index allocation
> for these structures should be taken care of by EDAC core.
> 

From: Andrew Morton <akpm@linux-foundation.org>

keep things neat.  Also avoids having global identifier device_index
shadowed by local identifier device_index.

Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Doug Thompson <norsk5@yahoo.com>
Cc: Harry Ciao <qingtao.cao@windriver.com>
Cc: Kumar Gala <galak@gate.crashing.org>
Cc: Michael Ellerman <michael@ellerman.id.au>
Cc: Paul Mackerras <paulus@samba.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 drivers/edac/edac_device.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff -puN drivers/edac/amd8111_edac.c~edac-add-edac_device_alloc_index-cleanup drivers/edac/amd8111_edac.c
diff -puN drivers/edac/edac_core.h~edac-add-edac_device_alloc_index-cleanup drivers/edac/edac_core.h
diff -puN drivers/edac/edac_device.c~edac-add-edac_device_alloc_index-cleanup drivers/edac/edac_device.c
--- a/drivers/edac/edac_device.c~edac-add-edac_device_alloc_index-cleanup
+++ a/drivers/edac/edac_device.c
@@ -37,7 +37,6 @@
  */
 static DEFINE_MUTEX(device_ctls_mutex);
 static LIST_HEAD(edac_device_list);
-static atomic_t device_indexes = ATOMIC_INIT(0);
 
 #ifdef CONFIG_EDAC_DEBUG
 static void edac_device_dump_device(struct edac_device_ctl_info *edac_dev)
@@ -499,6 +498,8 @@ void edac_device_reset_delay_period(stru
  */
 int edac_device_alloc_index(void)
 {
+	static atomic_t device_indexes = ATOMIC_INIT(0);
+
 	return atomic_inc_return(&device_indexes) - 1;
 }
 EXPORT_SYMBOL_GPL(edac_device_alloc_index);
_

^ permalink raw reply

* Re: [v2 PATCH 1/3] EDAC: Add CPC925 Memory Controller driver
From: Andrew Morton @ 2009-04-15 22:27 UTC (permalink / raw)
  To: Harry Ciao; +Cc: norsk5, bluesmoke-devel, linux-kernel, linuxppc-dev
In-Reply-To: <1239602716-18352-2-git-send-email-qingtao.cao@windriver.com>

On Mon, 13 Apr 2009 14:05:14 +0800
Harry Ciao <qingtao.cao@windriver.com> wrote:

> Introduce IBM CPC925 EDAC driver, which makes use of ECC, CPU and
> HyperTransport Link error detections and corrections on the IBM
> CPC925 Bridge and Memory Controller.

A wee cleanup:

--- a/drivers/edac/cpc925_edac.c~edac-add-cpc925-memory-controller-driver-cleanup
+++ a/drivers/edac/cpc925_edac.c
@@ -291,9 +291,6 @@ struct cpc925_dev_info {
 	void (*check)(struct edac_device_ctl_info *edac_dev);
 };
 
-
-static int edac_mc_idx;
-
 /* Get total memory size from Open Firmware DTB */
 static void get_total_mem(struct cpc925_mc_pdata *pdata)
 {
@@ -866,6 +863,7 @@ static int cpc925_mc_get_channels(void _
 
 static int __devinit cpc925_probe(struct platform_device *pdev)
 {
+	static int edac_mc_idx;
 	struct mem_ctl_info *mci;
 	void __iomem *vbase;
 	struct cpc925_mc_pdata *pdata;
_

^ permalink raw reply

* Re: [v2 PATCH 0/3] Add CPC925 Memory Controller EDAC drivers
From: Andrew Morton @ 2009-04-15 22:26 UTC (permalink / raw)
  To: Harry Ciao; +Cc: norsk5, bluesmoke-devel, linux-kernel, linuxppc-dev
In-Reply-To: <1239602716-18352-1-git-send-email-qingtao.cao@windriver.com>

On Mon, 13 Apr 2009 14:05:13 +0800
Harry Ciao <qingtao.cao@windriver.com> wrote:

> 
> Hi Doug and Michael,
> 
> This is the latest v2 patches, the 1/3 of CPC925 MC EDAC driver remains
> the same as before, the 2/3 has been pushed to Andrew already, and the 
> 3/3 has integrated Michael's suggestions to add a fixup routine for the 
> memory controller on Maple that has incorrect number of cells, and create
> related platform_device for CPC925 on Motorola ATCA-6101 target by far.
> 
> All patches have been tested, please give me your further concerns or 
> suggestions, thanks!

For some reason I didn't have 2/3 anywhere.

I'll plan to merge all these into 2.6.31 unless someone shouts at me.

^ permalink raw reply

* Re: [PATCH 05/14] phylib: add *_direct() variants of phy_connect and phy_attach functions
From: Andy Fleming @ 2009-04-15 21:10 UTC (permalink / raw)
  To: Grant Likely; +Cc: Joakim Tjernlund, netdev, linuxppc-dev, olof
In-Reply-To: <20090331082709.1427.48689.stgit@localhost.localdomain>


On Mar 31, 2009, at 3:27 AM, Grant Likely wrote:

> From: Grant Likely <grant.likely@secretlab.ca>
>
> Add phy_connect_direct() and phy_attach_direct() functions so that
> drivers can use a pointer to the phy_device instead of trying to  
> determine
> the phy's bus_id string.
>
> This patch is useful for OF device tree descriptions of phy devices  
> where
> the driver doesn't need or know what the bus_id value in order to  
> get a
> phy_device pointer.
>
> Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
> ---
> @@ -312,18 +339,21 @@ struct phy_device * phy_connect(struct  
> net_device *dev, const char *bus_id,
> 		phy_interface_t interface)
> {
> 	struct phy_device *phydev;
> +	struct device *d;
> +	int rc;
>
> -	phydev = phy_attach(dev, bus_id, flags, interface);
> -
> -	if (IS_ERR(phydev))
> -		return phydev;
> -
> -	phy_prepare_link(phydev, handler);
> -
> -	phy_start_machine(phydev, NULL);
> +	/* Search the list of PHY devices on the mdio bus for the
> +	 * PHY with the requested name */
> +	d = bus_find_device_by_name(&mdio_bus_type, NULL, bus_id);
> +	if (!d) {
> +		pr_err("PHY %s not found\n", bus_id);
> +		return ERR_PTR(-ENODEV);
> +	}
> +	phydev = to_phy_device(d);
>
> -	if (phydev->irq > 0)
> -		phy_start_interrupts(phydev);
> +	rc = phy_attach_direct(dev, phydev, flags, interface);
> +	if (rc)
> +		return ERR_PTR(rc);


Why not just invoke phy_attach(), here, and thereby avoid the  
duplicate search code?

Otherwise, looks good.

Andy

^ permalink raw reply

* [PATCH] powerpc: Refactor board check for quirk
From: Kumar Gala @ 2009-04-15 19:37 UTC (permalink / raw)
  To: linuxppc-dev

Refactor the check to determine if the quirk is applicable to the boards
into one inline function so we only have to change one place to add more
boards that the quirks might be applicable to.

Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
---
 arch/powerpc/platforms/fsl_uli1575.c |   22 ++++++++++++----------
 1 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/arch/powerpc/platforms/fsl_uli1575.c b/arch/powerpc/platforms/fsl_uli1575.c
index 1db6b9e..4e5ae0a 100644
--- a/arch/powerpc/platforms/fsl_uli1575.c
+++ b/arch/powerpc/platforms/fsl_uli1575.c
@@ -51,13 +51,19 @@ u8 uli_pirq_to_irq[8] = {
 	ULI_8259_NONE,		/* PIRQH */
 };
 
+static inline bool is_quirk_valid(void)
+{
+	return (machine_is(mpc86xx_hpcn) || 
+		machine_is(mpc8544_ds) ||
+		machine_is(mpc8572_ds))
+}
+
 /* Bridge */
 static void __devinit early_uli5249(struct pci_dev *dev)
 {
 	unsigned char temp;
 
-	if (!machine_is(mpc86xx_hpcn) && !machine_is(mpc8544_ds) &&
-			!machine_is(mpc8572_ds))
+	if (!is_quirk_valid())
 		return;
 
 	pci_write_config_word(dev, PCI_COMMAND, PCI_COMMAND_IO |
@@ -80,8 +86,7 @@ static void __devinit quirk_uli1575(struct pci_dev *dev)
 {
 	int i;
 
-	if (!machine_is(mpc86xx_hpcn) && !machine_is(mpc8544_ds) &&
-			!machine_is(mpc8572_ds))
+	if (!is_quirk_valid())
 		return;
 
 	/*
@@ -149,8 +154,7 @@ static void __devinit quirk_final_uli1575(struct pci_dev *dev)
 	 * IRQ 14: Edge
 	 * IRQ 15: Edge
 	 */
-	if (!machine_is(mpc86xx_hpcn) && !machine_is(mpc8544_ds) &&
-			!machine_is(mpc8572_ds))
+	if (!is_quirk_valid())
 		return;
 
 	outb(0xfa, 0x4d0);
@@ -176,8 +180,7 @@ static void __devinit quirk_uli5288(struct pci_dev *dev)
 	unsigned char c;
 	unsigned int d;
 
-	if (!machine_is(mpc86xx_hpcn) && !machine_is(mpc8544_ds) &&
-			!machine_is(mpc8572_ds))
+	if (!is_quirk_valid())
 		return;
 
 	/* read/write lock */
@@ -201,8 +204,7 @@ static void __devinit quirk_uli5229(struct pci_dev *dev)
 {
 	unsigned short temp;
 
-	if (!machine_is(mpc86xx_hpcn) && !machine_is(mpc8544_ds) &&
-			!machine_is(mpc8572_ds))
+	if (!is_quirk_valid())
 		return;
 
 	pci_write_config_word(dev, PCI_COMMAND, PCI_COMMAND_INTX_DISABLE |
-- 
1.6.0.6

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox