LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH v5 14/14] memory-hotplug: free node_data when a node is offlined
From: Wen Congyang @ 2012-12-27 12:16 UTC (permalink / raw)
  To: Kamezawa Hiroyuki
  Cc: linux-ia64, linux-sh, Tang Chen, linux-mm, paulus, hpa,
	sparclinux, cl, linux-s390, x86, linux-acpi, isimatu.yasuaki,
	linfeng, mgorman, kosaki.motohiro, rientjes, liuj97, len.brown,
	cmetcalf, wujianguo, yinghai, laijs, linux-kernel, minchan.kim,
	akpm, linuxppc-dev
In-Reply-To: <50DA7533.6060407@jp.fujitsu.com>

At 12/26/2012 11:55 AM, Kamezawa Hiroyuki Wrote:
> (2012/12/24 21:09), Tang Chen wrote:
>> From: Wen Congyang <wency@cn.fujitsu.com>
>>
>> We call hotadd_new_pgdat() to allocate memory to store node_data. So we
>> should free it when removing a node.
>>
>> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
> 
> I'm sorry but is it safe to remove pgdat ? All zone cache and zonelists are
> properly cleared/rebuilded in synchronous way ? and No threads are visinting
> zone in vmscan.c ?

We have rebuilt zonelists when a zone has no memory after offlining some pages.

Thanks
Wen Congyang

> 
> Thanks,
> -Kame
> 
>> ---
>>   mm/memory_hotplug.c |   20 +++++++++++++++++++-
>>   1 files changed, 19 insertions(+), 1 deletions(-)
>>
>> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
>> index f8a1d2f..447fa24 100644
>> --- a/mm/memory_hotplug.c
>> +++ b/mm/memory_hotplug.c
>> @@ -1680,9 +1680,12 @@ static int check_cpu_on_node(void *data)
>>   /* offline the node if all memory sections of this node are removed */
>>   static void try_offline_node(int nid)
>>   {
>> +	pg_data_t *pgdat = NODE_DATA(nid);
>>   	unsigned long start_pfn = NODE_DATA(nid)->node_start_pfn;
>> -	unsigned long end_pfn = start_pfn + NODE_DATA(nid)->node_spanned_pages;
>> +	unsigned long end_pfn = start_pfn + pgdat->node_spanned_pages;
>>   	unsigned long pfn;
>> +	struct page *pgdat_page = virt_to_page(pgdat);
>> +	int i;
>>   
>>   	for (pfn = start_pfn; pfn < end_pfn; pfn += PAGES_PER_SECTION) {
>>   		unsigned long section_nr = pfn_to_section_nr(pfn);
>> @@ -1709,6 +1712,21 @@ static void try_offline_node(int nid)
>>   	 */
>>   	node_set_offline(nid);
>>   	unregister_one_node(nid);
>> +
>> +	if (!PageSlab(pgdat_page) && !PageCompound(pgdat_page))
>> +		/* node data is allocated from boot memory */
>> +		return;
>> +
>> +	/* free waittable in each zone */
>> +	for (i = 0; i < MAX_NR_ZONES; i++) {
>> +		struct zone *zone = pgdat->node_zones + i;
>> +
>> +		if (zone->wait_table)
>> +			vfree(zone->wait_table);
>> +	}
>> +
>> +	arch_refresh_nodedata(nid, NULL);
>> +	arch_free_nodedata(pgdat);
>>   }
>>   
>>   int __ref remove_memory(int nid, u64 start, u64 size)
>>
> 
> 
> 

^ permalink raw reply

* [PATCH] PowerPC documentation: fixed path to the powerpc directory
From: Thomas Waldecker @ 2012-12-27 14:07 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Thomas Waldecker, linux-kernel, linux-doc

ppc -> powerpc

Signed-off-by: Thomas Waldecker <thomas.waldecker@gmail.com>
---
 Documentation/powerpc/cpu_features.txt | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/Documentation/powerpc/cpu_features.txt b/Documentation/powerpc/cpu_features.txt
index ffa4183..ae09df8 100644
--- a/Documentation/powerpc/cpu_features.txt
+++ b/Documentation/powerpc/cpu_features.txt
@@ -11,10 +11,10 @@ split instruction and data caches, and if the CPU supports the DOZE and NAP
 sleep modes.
 
 Detection of the feature set is simple. A list of processors can be found in
-arch/ppc/kernel/cputable.c. The PVR register is masked and compared with each
-value in the list. If a match is found, the cpu_features of cur_cpu_spec is
-assigned to the feature bitmask for this processor and a __setup_cpu function
-is called.
+arch/powerpc/kernel/cputable.c. The PVR register is masked and compared with
+each value in the list. If a match is found, the cpu_features of cur_cpu_spec
+is assigned to the feature bitmask for this processor and a __setup_cpu
+function is called.
 
 C code may test 'cur_cpu_spec[smp_processor_id()]->cpu_features' for a
 particular feature bit. This is done in quite a few places, for example
@@ -51,6 +51,6 @@ should be used in the majority of cases.
 
 The END_FTR_SECTION macros are implemented by storing information about this
 code in the '__ftr_fixup' ELF section. When do_cpu_ftr_fixups
-(arch/ppc/kernel/misc.S) is invoked, it will iterate over the records in
+(arch/powerpc/kernel/misc.S) is invoked, it will iterate over the records in
 __ftr_fixup, and if the required feature is not present it will loop writing
 nop's from each BEGIN_FTR_SECTION to END_FTR_SECTION.
-- 
1.8.0.2

^ permalink raw reply related

* [PATCH] ppc/iommu: prevent false TCE leak message
From: Thadeu Lima de Souza Cascardo @ 2012-12-27 16:28 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: paulus, anton, Thadeu Lima de Souza Cascardo

When a device DMA window includes the address 0, it's reserved in the
TCE bitmap to avoid returning that address to drivers.

When the device is removed, the bitmap is checked for any mappings not
removed by the driver, indicating a possible DMA mapping leak. Since the
reserved address is not cleared, a message is printed, warning of such a
leak.

Check for the reservation, and clear it before checking for any other
standing mappings.

Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@linux.vnet.ibm.com>
---
 arch/powerpc/kernel/iommu.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/kernel/iommu.c b/arch/powerpc/kernel/iommu.c
index 8226c6c..226e9e5 100644
--- a/arch/powerpc/kernel/iommu.c
+++ b/arch/powerpc/kernel/iommu.c
@@ -717,6 +717,11 @@ void iommu_free_table(struct iommu_table *tbl, const char *node_name)
 		return;
 	}
 
+	/* In case we have reserved the first bit, we should not emit
+	 * the warning below. */
+	if (tbl->it_offset == 0)
+		clear_bit(0, tbl->it_map);
+
 	/* verify that table contains no entries */
 	/* it_size is in entries, and we're examining 64 at a time */
 	for (i = 0; i < (tbl->it_size/64); i++) {
-- 
1.7.1

^ permalink raw reply related

* [PATCH] ppc/EEH: fix crash when adding a device in a slot with DDW
From: Thadeu Lima de Souza Cascardo @ 2012-12-27 16:34 UTC (permalink / raw)
  To: linuxppc-dev
  Cc: shangw, linux-kernel, stable, paulus,
	Thadeu Lima de Souza Cascardo, bhelgaas

The DDW code uses a eeh_dev struct from the pci_dev. However, this is
not set until eeh_add_device_late is called.

Since pci_bus_add_devices is called before eeh_add_device_late, the PCI
devices are added to the bus, making drivers' probe hooks to be called.
These will call set_dma_mask, which will call the DDW code, which will
require the eeh_dev struct from pci_dev. This would result in a crash,
due to a NULL dereference.

Calling eeh_add_device_late after pci_bus_add_devices would make the
system BUG, because device files shouldn't be added to devices there
were not added to the system. So, a new function is needed to add such
files only after pci_bus_add_devices have been called.

Cc: stable@vger.kernel.org
Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@linux.vnet.ibm.com>
---
 arch/powerpc/include/asm/eeh.h       |    3 +++
 arch/powerpc/kernel/pci-common.c     |    7 +++++--
 arch/powerpc/platforms/pseries/eeh.c |   24 +++++++++++++++++++++++-
 3 files changed, 31 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h
index b0ef738..71aac19 100644
--- a/arch/powerpc/include/asm/eeh.h
+++ b/arch/powerpc/include/asm/eeh.h
@@ -201,6 +201,7 @@ int eeh_dev_check_failure(struct eeh_dev *edev);
 void __init eeh_addr_cache_build(void);
 void eeh_add_device_tree_early(struct device_node *);
 void eeh_add_device_tree_late(struct pci_bus *);
+void eeh_add_device_tree_files(struct pci_bus *);
 void eeh_remove_bus_device(struct pci_dev *, int);
 
 /**
@@ -240,6 +241,8 @@ static inline void eeh_add_device_tree_early(struct device_node *dn) { }
 
 static inline void eeh_add_device_tree_late(struct pci_bus *bus) { }
 
+static inline void eeh_add_device_tree_files(struct pci_bus *bus) { }
+
 static inline void eeh_remove_bus_device(struct pci_dev *dev, int purge_pe) { }
 
 static inline void eeh_lock(void) { }
diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
index 7f94f76..7b1f14c 100644
--- a/arch/powerpc/kernel/pci-common.c
+++ b/arch/powerpc/kernel/pci-common.c
@@ -1480,11 +1480,14 @@ void pcibios_finish_adding_to_bus(struct pci_bus *bus)
 	pcibios_allocate_bus_resources(bus);
 	pcibios_claim_one_bus(bus);
 
+	/* Fixup EEH */
+	eeh_add_device_tree_late(bus);
+
 	/* Add new devices to global lists.  Register in proc, sysfs. */
 	pci_bus_add_devices(bus);
 
-	/* Fixup EEH */
-	eeh_add_device_tree_late(bus);
+	/* Add EEH sysfs files */
+	eeh_add_device_tree_files(bus);
 }
 EXPORT_SYMBOL_GPL(pcibios_finish_adding_to_bus);
 
diff --git a/arch/powerpc/platforms/pseries/eeh.c b/arch/powerpc/platforms/pseries/eeh.c
index 9a04322..a667a34 100644
--- a/arch/powerpc/platforms/pseries/eeh.c
+++ b/arch/powerpc/platforms/pseries/eeh.c
@@ -788,7 +788,6 @@ static void eeh_add_device_late(struct pci_dev *dev)
 	dev->dev.archdata.edev = edev;
 
 	eeh_addr_cache_insert_dev(dev);
-	eeh_sysfs_add_device(dev);
 }
 
 /**
@@ -815,6 +814,29 @@ void eeh_add_device_tree_late(struct pci_bus *bus)
 EXPORT_SYMBOL_GPL(eeh_add_device_tree_late);
 
 /**
+ * eeh_add_device_tree_files - Add EEH sysfs files for the indicated PCI bus
+ * @bus: PCI bus
+ *
+ * This routine must be used to add EEH sysfs files for PCI
+ * devices which are attached to the indicated PCI bus. The PCI bus
+ * is added after system boot through hotplug or dlpar.
+ */
+void eeh_add_device_tree_files(struct pci_bus *bus)
+{
+	struct pci_dev *dev;
+
+	list_for_each_entry(dev, &bus->devices, bus_list) {
+		eeh_sysfs_add_device(dev);
+		if (dev->hdr_type == PCI_HEADER_TYPE_BRIDGE) {
+			struct pci_bus *subbus = dev->subordinate;
+			if (subbus)
+				eeh_add_device_tree_files(subbus);
+		}
+	}
+}
+EXPORT_SYMBOL_GPL(eeh_add_device_tree_files);
+
+/**
  * eeh_remove_device - Undo EEH setup for the indicated pci device
  * @dev: pci device to be removed
  * @purge_pe: remove the PE or not
-- 
1.7.1

^ permalink raw reply related

* Re: [PATCH v5 14/14] memory-hotplug: free node_data when a node is offlined
From: Kamezawa Hiroyuki @ 2012-12-28  0:28 UTC (permalink / raw)
  To: Wen Congyang
  Cc: linux-ia64, linux-sh, Tang Chen, linux-mm, paulus, hpa,
	sparclinux, cl, linux-s390, x86, linux-acpi, isimatu.yasuaki,
	linfeng, mgorman, kosaki.motohiro, rientjes, liuj97, len.brown,
	cmetcalf, wujianguo, yinghai, laijs, linux-kernel, minchan.kim,
	akpm, linuxppc-dev
In-Reply-To: <50DC3C26.6060308@cn.fujitsu.com>

(2012/12/27 21:16), Wen Congyang wrote:
> At 12/26/2012 11:55 AM, Kamezawa Hiroyuki Wrote:
>> (2012/12/24 21:09), Tang Chen wrote:
>>> From: Wen Congyang <wency@cn.fujitsu.com>
>>>
>>> We call hotadd_new_pgdat() to allocate memory to store node_data. So we
>>> should free it when removing a node.
>>>
>>> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
>>
>> I'm sorry but is it safe to remove pgdat ? All zone cache and zonelists are
>> properly cleared/rebuilded in synchronous way ? and No threads are visinting
>> zone in vmscan.c ?
> 
> We have rebuilt zonelists when a zone has no memory after offlining some pages.
> 

How do you guarantee that the address of pgdat/zone is not on stack of any kernel
threads or other kernel objects without reference counting or other syncing method ?


Thanks,
-Kame

^ permalink raw reply

* Re: [PATCH] ppc/EEH: fix crash when adding a device in a slot with DDW
From: Gavin Shan @ 2012-12-28  5:18 UTC (permalink / raw)
  To: Thadeu Lima de Souza Cascardo
  Cc: shangw, linux-kernel, stable, paulus, bhelgaas, linuxppc-dev
In-Reply-To: <1356626040-9384-1-git-send-email-cascardo@linux.vnet.ibm.com>

On Thu, Dec 27, 2012 at 02:34:00PM -0200, Thadeu Lima de Souza Cascardo wrote:
>The DDW code uses a eeh_dev struct from the pci_dev. However, this is
>not set until eeh_add_device_late is called.
>
>Since pci_bus_add_devices is called before eeh_add_device_late, the PCI
>devices are added to the bus, making drivers' probe hooks to be called.
>These will call set_dma_mask, which will call the DDW code, which will
>require the eeh_dev struct from pci_dev. This would result in a crash,
>due to a NULL dereference.
>
>Calling eeh_add_device_late after pci_bus_add_devices would make the
>system BUG, because device files shouldn't be added to devices there
>were not added to the system. So, a new function is needed to add such
>files only after pci_bus_add_devices have been called.
>

Could you please explain for a bit how did you trigger the problem? I'm
not sure you got it while doing PCI hotplug or just saw the issue during
system bootup stage :-)

>Cc: stable@vger.kernel.org
>Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@linux.vnet.ibm.com>
>---
> arch/powerpc/include/asm/eeh.h       |    3 +++
> arch/powerpc/kernel/pci-common.c     |    7 +++++--
> arch/powerpc/platforms/pseries/eeh.c |   24 +++++++++++++++++++++++-
> 3 files changed, 31 insertions(+), 3 deletions(-)
>
>diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h
>index b0ef738..71aac19 100644
>--- a/arch/powerpc/include/asm/eeh.h
>+++ b/arch/powerpc/include/asm/eeh.h
>@@ -201,6 +201,7 @@ int eeh_dev_check_failure(struct eeh_dev *edev);
> void __init eeh_addr_cache_build(void);
> void eeh_add_device_tree_early(struct device_node *);
> void eeh_add_device_tree_late(struct pci_bus *);
>+void eeh_add_device_tree_files(struct pci_bus *);

Since the function is going to add EEH specific sysfs files, its name would
be something like "eeh_add_sysfs_files" instead of "eeh_add_device_tree_files" :-)

> void eeh_remove_bus_device(struct pci_dev *, int);
>
> /**
>@@ -240,6 +241,8 @@ static inline void eeh_add_device_tree_early(struct device_node *dn) { }
>
> static inline void eeh_add_device_tree_late(struct pci_bus *bus) { }
>
>+static inline void eeh_add_device_tree_files(struct pci_bus *bus) { }
>+

It'd better to rename the function name to "eeh_add_sysfs_files" mentioned
as above.

> static inline void eeh_remove_bus_device(struct pci_dev *dev, int purge_pe) { }
>
> static inline void eeh_lock(void) { }
>diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
>index 7f94f76..7b1f14c 100644
>--- a/arch/powerpc/kernel/pci-common.c
>+++ b/arch/powerpc/kernel/pci-common.c
>@@ -1480,11 +1480,14 @@ void pcibios_finish_adding_to_bus(struct pci_bus *bus)
> 	pcibios_allocate_bus_resources(bus);
> 	pcibios_claim_one_bus(bus);
>
>+	/* Fixup EEH */
>+	eeh_add_device_tree_late(bus);
>+
> 	/* Add new devices to global lists.  Register in proc, sysfs. */
> 	pci_bus_add_devices(bus);
>
>-	/* Fixup EEH */
>-	eeh_add_device_tree_late(bus);
>+	/* Add EEH sysfs files */
>+	eeh_add_device_tree_files(bus);

The function name would be "eeh_add_sysfs_files" as above.

> }
> EXPORT_SYMBOL_GPL(pcibios_finish_adding_to_bus);
>

By the way, arch/powerpc/kernel/of_platform.c::of_pci_phb_probe is also calling
to eeh_add_device_tree_late() as well. Since you have removed part of the logic
from original eeh_add_device_tree_late(), which is add EEH specific sysfs files,
and you put that part of logic to eeh_add_device_tree_files(). So I think you
also need make the similiar change for of_pci_phb_probe() as well :-)

>diff --git a/arch/powerpc/platforms/pseries/eeh.c b/arch/powerpc/platforms/pseries/eeh.c
>index 9a04322..a667a34 100644
>--- a/arch/powerpc/platforms/pseries/eeh.c
>+++ b/arch/powerpc/platforms/pseries/eeh.c
>@@ -788,7 +788,6 @@ static void eeh_add_device_late(struct pci_dev *dev)
> 	dev->dev.archdata.edev = edev;
>
> 	eeh_addr_cache_insert_dev(dev);
>-	eeh_sysfs_add_device(dev);
> }
>
> /**
>@@ -815,6 +814,29 @@ void eeh_add_device_tree_late(struct pci_bus *bus)
> EXPORT_SYMBOL_GPL(eeh_add_device_tree_late);
>
> /**
>+ * eeh_add_device_tree_files - Add EEH sysfs files for the indicated PCI bus
>+ * @bus: PCI bus
>+ *
>+ * This routine must be used to add EEH sysfs files for PCI
>+ * devices which are attached to the indicated PCI bus. The PCI bus
>+ * is added after system boot through hotplug or dlpar.
>+ */
>+void eeh_add_device_tree_files(struct pci_bus *bus)
>+{
>+	struct pci_dev *dev;
>+
>+	list_for_each_entry(dev, &bus->devices, bus_list) {
>+		eeh_sysfs_add_device(dev);
>+		if (dev->hdr_type == PCI_HEADER_TYPE_BRIDGE) {
>+			struct pci_bus *subbus = dev->subordinate;
>+			if (subbus)
>+				eeh_add_device_tree_files(subbus);
>+		}
>+	}
>+}
>+EXPORT_SYMBOL_GPL(eeh_add_device_tree_files);
>+

The function name mentioned as above.

>+/**
>  * eeh_remove_device - Undo EEH setup for the indicated pci device
>  * @dev: pci device to be removed
>  * @purge_pe: remove the PE or not
>

Thanks,
Gavin

^ permalink raw reply

* Re: [PATCH] ppc/iommu: prevent false TCE leak message
From: Gavin Shan @ 2012-12-28  5:21 UTC (permalink / raw)
  To: Thadeu Lima de Souza Cascardo; +Cc: paulus, linuxppc-dev, anton
In-Reply-To: <1356625686-8943-1-git-send-email-cascardo@linux.vnet.ibm.com>

On Thu, Dec 27, 2012 at 02:28:06PM -0200, Thadeu Lima de Souza Cascardo wrote:
>When a device DMA window includes the address 0, it's reserved in the
>TCE bitmap to avoid returning that address to drivers.
>
>When the device is removed, the bitmap is checked for any mappings not
>removed by the driver, indicating a possible DMA mapping leak. Since the
>reserved address is not cleared, a message is printed, warning of such a
>leak.
>
>Check for the reservation, and clear it before checking for any other
>standing mappings.
>
>Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@linux.vnet.ibm.com>
>---
> arch/powerpc/kernel/iommu.c |    5 +++++
> 1 files changed, 5 insertions(+), 0 deletions(-)
>
>diff --git a/arch/powerpc/kernel/iommu.c b/arch/powerpc/kernel/iommu.c
>index 8226c6c..226e9e5 100644
>--- a/arch/powerpc/kernel/iommu.c
>+++ b/arch/powerpc/kernel/iommu.c
>@@ -717,6 +717,11 @@ void iommu_free_table(struct iommu_table *tbl, const char *node_name)
> 		return;
> 	}
>
>+	/* In case we have reserved the first bit, we should not emit
>+	 * the warning below. */

At least, the comment would look like:

	/*
	 * xxxxxxx
	 */

>+	if (tbl->it_offset == 0)
>+		clear_bit(0, tbl->it_map);
>+
> 	/* verify that table contains no entries */
> 	/* it_size is in entries, and we're examining 64 at a time */

The comment would be merged as well? :-)

> 	for (i = 0; i < (tbl->it_size/64); i++) {

Thanks,
Gavin

^ permalink raw reply

* Re: [PATCH] ppc/EEH: fix crash when adding a device in a slot with DDW
From: Thadeu Lima de Souza Cascardo @ 2012-12-28 12:06 UTC (permalink / raw)
  To: Gavin Shan; +Cc: linux-kernel, stable, paulus, bhelgaas, linuxppc-dev
In-Reply-To: <20121228051824.GA9975@shangw.(null)>

On Fri, Dec 28, 2012 at 01:18:24PM +0800, Gavin Shan wrote:
> On Thu, Dec 27, 2012 at 02:34:00PM -0200, Thadeu Lima de Souza Cascardo wrote:
> >The DDW code uses a eeh_dev struct from the pci_dev. However, this is
> >not set until eeh_add_device_late is called.
> >
> >Since pci_bus_add_devices is called before eeh_add_device_late, the PCI
> >devices are added to the bus, making drivers' probe hooks to be called.
> >These will call set_dma_mask, which will call the DDW code, which will
> >require the eeh_dev struct from pci_dev. This would result in a crash,
> >due to a NULL dereference.
> >
> >Calling eeh_add_device_late after pci_bus_add_devices would make the
> >system BUG, because device files shouldn't be added to devices there
> >were not added to the system. So, a new function is needed to add such
> >files only after pci_bus_add_devices have been called.
> >
> 
> Could you please explain for a bit how did you trigger the problem? I'm
> not sure you got it while doing PCI hotplug or just saw the issue during
> system bootup stage :-)
> 

I did a DLPAR remove followed by a DLPAR add, using drmgr -r and drmgr
-a. The reason the issue is not trigerred during bootup is that there is
no driver registered yet. So no driver probe will call dma_set_mask.

> >Cc: stable@vger.kernel.org
> >Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@linux.vnet.ibm.com>
> >---
> > arch/powerpc/include/asm/eeh.h       |    3 +++
> > arch/powerpc/kernel/pci-common.c     |    7 +++++--
> > arch/powerpc/platforms/pseries/eeh.c |   24 +++++++++++++++++++++++-
> > 3 files changed, 31 insertions(+), 3 deletions(-)
> >
> >diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h
> >index b0ef738..71aac19 100644
> >--- a/arch/powerpc/include/asm/eeh.h
> >+++ b/arch/powerpc/include/asm/eeh.h
> >@@ -201,6 +201,7 @@ int eeh_dev_check_failure(struct eeh_dev *edev);
> > void __init eeh_addr_cache_build(void);
> > void eeh_add_device_tree_early(struct device_node *);
> > void eeh_add_device_tree_late(struct pci_bus *);
> >+void eeh_add_device_tree_files(struct pci_bus *);
> 
> Since the function is going to add EEH specific sysfs files, its name would
> be something like "eeh_add_sysfs_files" instead of "eeh_add_device_tree_files" :-)

It's indeed a better name.

> 
> > void eeh_remove_bus_device(struct pci_dev *, int);
> >
> > /**
> >@@ -240,6 +241,8 @@ static inline void eeh_add_device_tree_early(struct device_node *dn) { }
> >
> > static inline void eeh_add_device_tree_late(struct pci_bus *bus) { }
> >
> >+static inline void eeh_add_device_tree_files(struct pci_bus *bus) { }
> >+
> 
> It'd better to rename the function name to "eeh_add_sysfs_files" mentioned
> as above.
> 
> > static inline void eeh_remove_bus_device(struct pci_dev *dev, int purge_pe) { }
> >
> > static inline void eeh_lock(void) { }
> >diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
> >index 7f94f76..7b1f14c 100644
> >--- a/arch/powerpc/kernel/pci-common.c
> >+++ b/arch/powerpc/kernel/pci-common.c
> >@@ -1480,11 +1480,14 @@ void pcibios_finish_adding_to_bus(struct pci_bus *bus)
> > 	pcibios_allocate_bus_resources(bus);
> > 	pcibios_claim_one_bus(bus);
> >
> >+	/* Fixup EEH */
> >+	eeh_add_device_tree_late(bus);
> >+
> > 	/* Add new devices to global lists.  Register in proc, sysfs. */
> > 	pci_bus_add_devices(bus);
> >
> >-	/* Fixup EEH */
> >-	eeh_add_device_tree_late(bus);
> >+	/* Add EEH sysfs files */
> >+	eeh_add_device_tree_files(bus);
> 
> The function name would be "eeh_add_sysfs_files" as above.
> 
> > }
> > EXPORT_SYMBOL_GPL(pcibios_finish_adding_to_bus);
> >
> 
> By the way, arch/powerpc/kernel/of_platform.c::of_pci_phb_probe is also calling
> to eeh_add_device_tree_late() as well. Since you have removed part of the logic
> from original eeh_add_device_tree_late(), which is add EEH specific sysfs files,
> and you put that part of logic to eeh_add_device_tree_files(). So I think you
> also need make the similiar change for of_pci_phb_probe() as well :-)
> 

Good point. I will look into other call sites. However, I don't have
access to other platforms to test the patch.

> >diff --git a/arch/powerpc/platforms/pseries/eeh.c b/arch/powerpc/platforms/pseries/eeh.c
> >index 9a04322..a667a34 100644
> >--- a/arch/powerpc/platforms/pseries/eeh.c
> >+++ b/arch/powerpc/platforms/pseries/eeh.c
> >@@ -788,7 +788,6 @@ static void eeh_add_device_late(struct pci_dev *dev)
> > 	dev->dev.archdata.edev = edev;
> >
> > 	eeh_addr_cache_insert_dev(dev);
> >-	eeh_sysfs_add_device(dev);
> > }
> >
> > /**
> >@@ -815,6 +814,29 @@ void eeh_add_device_tree_late(struct pci_bus *bus)
> > EXPORT_SYMBOL_GPL(eeh_add_device_tree_late);
> >
> > /**
> >+ * eeh_add_device_tree_files - Add EEH sysfs files for the indicated PCI bus
> >+ * @bus: PCI bus
> >+ *
> >+ * This routine must be used to add EEH sysfs files for PCI
> >+ * devices which are attached to the indicated PCI bus. The PCI bus
> >+ * is added after system boot through hotplug or dlpar.
> >+ */
> >+void eeh_add_device_tree_files(struct pci_bus *bus)
> >+{
> >+	struct pci_dev *dev;
> >+
> >+	list_for_each_entry(dev, &bus->devices, bus_list) {
> >+		eeh_sysfs_add_device(dev);
> >+		if (dev->hdr_type == PCI_HEADER_TYPE_BRIDGE) {
> >+			struct pci_bus *subbus = dev->subordinate;
> >+			if (subbus)
> >+				eeh_add_device_tree_files(subbus);
> >+		}
> >+	}
> >+}
> >+EXPORT_SYMBOL_GPL(eeh_add_device_tree_files);
> >+
> 
> The function name mentioned as above.
> 
> >+/**
> >  * eeh_remove_device - Undo EEH setup for the indicated pci device
> >  * @dev: pci device to be removed
> >  * @purge_pe: remove the PE or not
> >
> 
> Thanks,
> Gavin
> 

Regards,
Thadeu Cascardo.

^ permalink raw reply

* Re: [PATCH] ppc/iommu: prevent false TCE leak message
From: Thadeu Lima de Souza Cascardo @ 2012-12-28 16:55 UTC (permalink / raw)
  To: Gavin Shan; +Cc: paulus, linuxppc-dev, anton
In-Reply-To: <20121228052135.GB9975@shangw.(null)>

On Fri, Dec 28, 2012 at 01:21:35PM +0800, Gavin Shan wrote:
> On Thu, Dec 27, 2012 at 02:28:06PM -0200, Thadeu Lima de Souza Cascardo wrote:
> >When a device DMA window includes the address 0, it's reserved in the
> >TCE bitmap to avoid returning that address to drivers.
> >
> >When the device is removed, the bitmap is checked for any mappings not
> >removed by the driver, indicating a possible DMA mapping leak. Since the
> >reserved address is not cleared, a message is printed, warning of such a
> >leak.
> >
> >Check for the reservation, and clear it before checking for any other
> >standing mappings.
> >
> >Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@linux.vnet.ibm.com>
> >---
> > arch/powerpc/kernel/iommu.c |    5 +++++
> > 1 files changed, 5 insertions(+), 0 deletions(-)
> >
> >diff --git a/arch/powerpc/kernel/iommu.c b/arch/powerpc/kernel/iommu.c
> >index 8226c6c..226e9e5 100644
> >--- a/arch/powerpc/kernel/iommu.c
> >+++ b/arch/powerpc/kernel/iommu.c
> >@@ -717,6 +717,11 @@ void iommu_free_table(struct iommu_table *tbl, const char *node_name)
> > 		return;
> > 	}
> >
> >+	/* In case we have reserved the first bit, we should not emit
> >+	 * the warning below. */
> 
> At least, the comment would look like:
> 
> 	/*
> 	 * xxxxxxx
> 	 */
> 

Sure. New code should always follow coding style.  :-)

How do you suggest merging with the comment below? I think it's closer
to the code it comments about, so I'd rather keep it where it is.

> >+	if (tbl->it_offset == 0)
> >+		clear_bit(0, tbl->it_map);
> >+
> > 	/* verify that table contains no entries */
> > 	/* it_size is in entries, and we're examining 64 at a time */
> 
> The comment would be merged as well? :-)
> 

I also considered replacing this code by this:

        /* verify that table contains no entries */
-       /* it_size is in entries, and we're examining 64 at a time */
-       for (i = 0; i < (tbl->it_size/64); i++) {
-               if (tbl->it_map[i] != 0) {
+       if (find_first_bit(tbl->it_map, tbl->it_size) < tbl->it_size)
                        printk(KERN_WARNING "%s: Unexpected TCEs for
%s\n",
                                __func__, node_name);
-                       break;
-               }
-       }
 
I'll resend the "unreserving" patch with the fixed comment styling, but
without merging comments. And I will send this other patch for comments.

Regards.
Thadeu Cascardo.

 
> > 	for (i = 0; i < (tbl->it_size/64); i++) {
> 
> Thanks,
> Gavin

^ permalink raw reply

* [PATCH 1/2] ppc/iommu: prevent false TCE leak message
From: Thadeu Lima de Souza Cascardo @ 2012-12-28 19:08 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: paulus, shangw, Thadeu Lima de Souza Cascardo, anton
In-Reply-To: <20121228165504.GC21831@oc0268524204.ibm.com>

When a device DMA window includes the address 0, it's reserved in the
TCE bitmap to avoid returning that address to drivers.

When the device is removed, the bitmap is checked for any mappings not
removed by the driver, indicating a possible DMA mapping leak. Since the
reserved address is not cleared, a message is printed, warning of such a
leak.

Check for the reservation, and clear it before checking for any other
standing mappings.

Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@linux.vnet.ibm.com>
---
 arch/powerpc/kernel/iommu.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/kernel/iommu.c b/arch/powerpc/kernel/iommu.c
index 8226c6c..6d48ff8 100644
--- a/arch/powerpc/kernel/iommu.c
+++ b/arch/powerpc/kernel/iommu.c
@@ -717,6 +717,13 @@ void iommu_free_table(struct iommu_table *tbl, const char *node_name)
 		return;
 	}
 
+	/*
+	 * In case we have reserved the first bit, we should not emit
+	 * the warning below.
+	 */
+	if (tbl->it_offset == 0)
+		clear_bit(0, tbl->it_map);
+
 	/* verify that table contains no entries */
 	/* it_size is in entries, and we're examining 64 at a time */
 	for (i = 0; i < (tbl->it_size/64); i++) {
-- 
1.7.1

^ permalink raw reply related

* [PATCH 2/2] ppc/iommu: use find_first_bit to look up entries in the iommu table
From: Thadeu Lima de Souza Cascardo @ 2012-12-28 19:08 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: paulus, shangw, Thadeu Lima de Souza Cascardo, anton
In-Reply-To: <20121228165504.GC21831@oc0268524204.ibm.com>

Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@linux.vnet.ibm.com>
---
 arch/powerpc/kernel/iommu.c |    7 +------
 1 files changed, 1 insertions(+), 6 deletions(-)

diff --git a/arch/powerpc/kernel/iommu.c b/arch/powerpc/kernel/iommu.c
index 6d48ff8..91e2b99 100644
--- a/arch/powerpc/kernel/iommu.c
+++ b/arch/powerpc/kernel/iommu.c
@@ -725,14 +725,9 @@ void iommu_free_table(struct iommu_table *tbl, const char *node_name)
 		clear_bit(0, tbl->it_map);
 
 	/* verify that table contains no entries */
-	/* it_size is in entries, and we're examining 64 at a time */
-	for (i = 0; i < (tbl->it_size/64); i++) {
-		if (tbl->it_map[i] != 0) {
+	if (find_first_bit(tbl->it_map, tbl->it_size) < tbl->it_size)
 			printk(KERN_WARNING "%s: Unexpected TCEs for %s\n",
 				__func__, node_name);
-			break;
-		}
-	}
 
 	/* calculate bitmap size in bytes */
 	bitmap_sz = (tbl->it_size + 7) / 8;
-- 
1.7.1

^ permalink raw reply related

* [PATCH 2/2] ppc/EEH: fix crash when adding a device in a slot with DDW
From: Thadeu Lima de Souza Cascardo @ 2012-12-28 19:13 UTC (permalink / raw)
  To: linuxppc-dev
  Cc: shangw, linux-kernel, stable, paulus,
	Thadeu Lima de Souza Cascardo, bhelgaas
In-Reply-To: <20121228120616.GB21831@oc0268524204.ibm.com>

The DDW code uses a eeh_dev struct from the pci_dev. However, this is
not set until eeh_add_device_late is called.

Since pci_bus_add_devices is called before eeh_add_device_late, the PCI
devices are added to the bus, making drivers' probe hooks to be called.
These will call set_dma_mask, which will call the DDW code, which will
require the eeh_dev struct from pci_dev. This would result in a crash,
due to a NULL dereference.

Calling eeh_add_device_late after pci_bus_add_devices would make the
system BUG, because device files shouldn't be added to devices there
were not added to the system. So, a new function is needed to add such
files only after pci_bus_add_devices have been called.

Cc: stable@vger.kernel.org
Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@linux.vnet.ibm.com>
---
 arch/powerpc/include/asm/eeh.h       |    3 +++
 arch/powerpc/kernel/of_platform.c    |    3 +++
 arch/powerpc/kernel/pci-common.c     |    7 +++++--
 arch/powerpc/platforms/pseries/eeh.c |   24 +++++++++++++++++++++++-
 4 files changed, 34 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h
index b0ef738..0f816da 100644
--- a/arch/powerpc/include/asm/eeh.h
+++ b/arch/powerpc/include/asm/eeh.h
@@ -201,6 +201,7 @@ int eeh_dev_check_failure(struct eeh_dev *edev);
 void __init eeh_addr_cache_build(void);
 void eeh_add_device_tree_early(struct device_node *);
 void eeh_add_device_tree_late(struct pci_bus *);
+void eeh_add_sysfs_files(struct pci_bus *);
 void eeh_remove_bus_device(struct pci_dev *, int);
 
 /**
@@ -240,6 +241,8 @@ static inline void eeh_add_device_tree_early(struct device_node *dn) { }
 
 static inline void eeh_add_device_tree_late(struct pci_bus *bus) { }
 
+static inline void eeh_add_sysfs_files(struct pci_bus *bus) { }
+
 static inline void eeh_remove_bus_device(struct pci_dev *dev, int purge_pe) { }
 
 static inline void eeh_lock(void) { }
diff --git a/arch/powerpc/kernel/of_platform.c b/arch/powerpc/kernel/of_platform.c
index c5fc6b2..500dd32 100644
--- a/arch/powerpc/kernel/of_platform.c
+++ b/arch/powerpc/kernel/of_platform.c
@@ -91,6 +91,9 @@ static int __devinit of_pci_phb_probe(struct platform_device *dev)
 	/* Add probed PCI devices to the device model */
 	pci_bus_add_devices(phb->bus);
 
+	/* sysfs files should only be added after devices are added */
+	eeh_add_sysfs_files(phb->bus);
+
 	return 0;
 }
 
diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
index 7f94f76..4d3de7e 100644
--- a/arch/powerpc/kernel/pci-common.c
+++ b/arch/powerpc/kernel/pci-common.c
@@ -1480,11 +1480,14 @@ void pcibios_finish_adding_to_bus(struct pci_bus *bus)
 	pcibios_allocate_bus_resources(bus);
 	pcibios_claim_one_bus(bus);
 
+	/* Fixup EEH */
+	eeh_add_device_tree_late(bus);
+
 	/* Add new devices to global lists.  Register in proc, sysfs. */
 	pci_bus_add_devices(bus);
 
-	/* Fixup EEH */
-	eeh_add_device_tree_late(bus);
+	/* sysfs files should only be added after devices are added */
+	eeh_add_sysfs_files(bus);
 }
 EXPORT_SYMBOL_GPL(pcibios_finish_adding_to_bus);
 
diff --git a/arch/powerpc/platforms/pseries/eeh.c b/arch/powerpc/platforms/pseries/eeh.c
index 9a04322..6b73d6c 100644
--- a/arch/powerpc/platforms/pseries/eeh.c
+++ b/arch/powerpc/platforms/pseries/eeh.c
@@ -788,7 +788,6 @@ static void eeh_add_device_late(struct pci_dev *dev)
 	dev->dev.archdata.edev = edev;
 
 	eeh_addr_cache_insert_dev(dev);
-	eeh_sysfs_add_device(dev);
 }
 
 /**
@@ -815,6 +814,29 @@ void eeh_add_device_tree_late(struct pci_bus *bus)
 EXPORT_SYMBOL_GPL(eeh_add_device_tree_late);
 
 /**
+ * eeh_add_sysfs_files - Add EEH sysfs files for the indicated PCI bus
+ * @bus: PCI bus
+ *
+ * This routine must be used to add EEH sysfs files for PCI
+ * devices which are attached to the indicated PCI bus. The PCI bus
+ * is added after system boot through hotplug or dlpar.
+ */
+void eeh_add_sysfs_files(struct pci_bus *bus)
+{
+	struct pci_dev *dev;
+
+	list_for_each_entry(dev, &bus->devices, bus_list) {
+		eeh_sysfs_add_device(dev);
+		if (dev->hdr_type == PCI_HEADER_TYPE_BRIDGE) {
+			struct pci_bus *subbus = dev->subordinate;
+			if (subbus)
+				eeh_add_sysfs_files(subbus);
+		}
+	}
+}
+EXPORT_SYMBOL_GPL(eeh_add_sysfs_files);
+
+/**
  * eeh_remove_device - Undo EEH setup for the indicated pci device
  * @dev: pci device to be removed
  * @purge_pe: remove the PE or not
-- 
1.7.1

^ permalink raw reply related

* [PATCH 1/2] EEH/OF: checking for CONFIG_EEH is not needed
From: Thadeu Lima de Souza Cascardo @ 2012-12-28 19:13 UTC (permalink / raw)
  To: linuxppc-dev
  Cc: bhelgaas, paulus, linux-kernel, Thadeu Lima de Souza Cascardo,
	shangw
In-Reply-To: <20121228120616.GB21831@oc0268524204.ibm.com>

The functions used are already defined as empty inline functions for the
case where EEH is disabled.

Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@linux.vnet.ibm.com>
---
 arch/powerpc/kernel/of_platform.c |    4 ----
 1 files changed, 0 insertions(+), 4 deletions(-)

diff --git a/arch/powerpc/kernel/of_platform.c b/arch/powerpc/kernel/of_platform.c
index 2049f2d..c5fc6b2 100644
--- a/arch/powerpc/kernel/of_platform.c
+++ b/arch/powerpc/kernel/of_platform.c
@@ -71,10 +71,8 @@ static int __devinit of_pci_phb_probe(struct platform_device *dev)
 	eeh_dev_phb_init_dynamic(phb);
 
 	/* Register devices with EEH */
-#ifdef CONFIG_EEH
 	if (dev->dev.of_node->child)
 		eeh_add_device_tree_early(dev->dev.of_node);
-#endif /* CONFIG_EEH */
 
 	/* Scan the bus */
 	pcibios_scan_phb(phb);
@@ -88,9 +86,7 @@ static int __devinit of_pci_phb_probe(struct platform_device *dev)
 	pcibios_claim_one_bus(phb->bus);
 
 	/* Finish EEH setup */
-#ifdef CONFIG_EEH
 	eeh_add_device_tree_late(phb->bus);
-#endif
 
 	/* Add probed PCI devices to the device model */
 	pci_bus_add_devices(phb->bus);
-- 
1.7.1

^ permalink raw reply related

* Re: [PATCH v5 01/14] memory-hotplug: try to offline the memory twice to avoid dependence
From: Wen Congyang @ 2012-12-30  5:49 UTC (permalink / raw)
  To: Kamezawa Hiroyuki
  Cc: linux-ia64, linux-sh, Tang Chen, linux-mm, paulus, hpa,
	sparclinux, cl, linux-s390, x86, linux-acpi, isimatu.yasuaki,
	linfeng, mgorman, kosaki.motohiro, rientjes, liuj97, len.brown,
	cmetcalf, wujianguo, yinghai, laijs, linux-kernel, minchan.kim,
	akpm, linuxppc-dev
In-Reply-To: <50DA68A8.4060001@jp.fujitsu.com>

At 12/26/2012 11:02 AM, Kamezawa Hiroyuki Wrote:
> (2012/12/24 21:09), Tang Chen wrote:
>> From: Wen Congyang <wency@cn.fujitsu.com>
>>
>> memory can't be offlined when CONFIG_MEMCG is selected.
>> For example: there is a memory device on node 1. The address range
>> is [1G, 1.5G). You will find 4 new directories memory8, memory9, memory10,
>> and memory11 under the directory /sys/devices/system/memory/.
>>
>> If CONFIG_MEMCG is selected, we will allocate memory to store page cgroup
>> when we online pages. When we online memory8, the memory stored page cgroup
>> is not provided by this memory device. But when we online memory9, the memory
>> stored page cgroup may be provided by memory8. So we can't offline memory8
>> now. We should offline the memory in the reversed order.
>>
> 
> If memory8 is onlined as NORMAL memory ...right ?

Yes, memory8 is onlined as NORMAL memory. And when we online memory9, we allocate
memory from memory8 to store page cgroup information.

> 
> IIUC, vmalloc() uses __GFP_HIGHMEM but doesn't use __GFP_MOVABLE.
> 
>> When the memory device is hotremoved, we will auto offline memory provided
>> by this memory device. But we don't know which memory is onlined first, so
>> offlining memory may fail. In such case, iterate twice to offline the memory.
>> 1st iterate: offline every non primary memory block.
>> 2nd iterate: offline primary (i.e. first added) memory block.
>>
>> This idea is suggested by KOSAKI Motohiro.
>>
>> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
> 
> I'm not sure but the whole DIMM should be onlined as MOVABLE mem ?

If the whole DIMM is onlined as MOVABLE mem, we can offline it, and don't
retry again.

> 
> Anyway, I agree this kind of retry is required if memory is onlined as NORMAL mem.
> But retry-once is ok ?

I'am not sure, but I think in most cases the user may online the memory according first
which is hot-added first. So we may always fail in the first time, and retry-once can
success.

Thanks
Wen Congyang

> 
> Thanks,
> -Kame
> 
>> ---
>>   mm/memory_hotplug.c |   16 ++++++++++++++--
>>   1 files changed, 14 insertions(+), 2 deletions(-)
>>
>> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
>> index d04ed87..62e04c9 100644
>> --- a/mm/memory_hotplug.c
>> +++ b/mm/memory_hotplug.c
>> @@ -1388,10 +1388,13 @@ int remove_memory(u64 start, u64 size)
>>   	unsigned long start_pfn, end_pfn;
>>   	unsigned long pfn, section_nr;
>>   	int ret;
>> +	int return_on_error = 0;
>> +	int retry = 0;
>>   
>>   	start_pfn = PFN_DOWN(start);
>>   	end_pfn = start_pfn + PFN_DOWN(size);
>>   
>> +repeat:
>>   	for (pfn = start_pfn; pfn < end_pfn; pfn += PAGES_PER_SECTION) {
>>   		section_nr = pfn_to_section_nr(pfn);
>>   		if (!present_section_nr(section_nr))
>> @@ -1410,14 +1413,23 @@ int remove_memory(u64 start, u64 size)
>>   
>>   		ret = offline_memory_block(mem);
>>   		if (ret) {
>> -			kobject_put(&mem->dev.kobj);
>> -			return ret;
>> +			if (return_on_error) {
>> +				kobject_put(&mem->dev.kobj);
>> +				return ret;
>> +			} else {
>> +				retry = 1;
>> +			}
>>   		}
>>   	}
>>   
>>   	if (mem)
>>   		kobject_put(&mem->dev.kobj);
>>   
>> +	if (retry) {
>> +		return_on_error = 1;
>> +		goto repeat;
>> +	}
>> +
>>   	return 0;
>>   }
>>   #else
>>
> 
> 
> 

^ permalink raw reply

* Re: [PATCH v5 01/14] memory-hotplug: try to offline the memory twice to avoid dependence
From: Wen Congyang @ 2012-12-30  5:58 UTC (permalink / raw)
  To: Glauber Costa
  Cc: linux-ia64, linux-sh, Tang Chen, linux-mm, paulus, hpa,
	sparclinux, cl, linux-s390, x86, linux-acpi, isimatu.yasuaki,
	linfeng, mgorman, kosaki.motohiro, rientjes, liuj97, len.brown,
	cmetcalf, wujianguo, yinghai, laijs, linux-kernel, minchan.kim,
	akpm, linuxppc-dev
In-Reply-To: <50D96543.6010903@parallels.com>

At 12/25/2012 04:35 PM, Glauber Costa Wrote:
> On 12/24/2012 04:09 PM, Tang Chen wrote:
>> From: Wen Congyang <wency@cn.fujitsu.com>
>>
>> memory can't be offlined when CONFIG_MEMCG is selected.
>> For example: there is a memory device on node 1. The address range
>> is [1G, 1.5G). You will find 4 new directories memory8, memory9, memory10,
>> and memory11 under the directory /sys/devices/system/memory/.
>>
>> If CONFIG_MEMCG is selected, we will allocate memory to store page cgroup
>> when we online pages. When we online memory8, the memory stored page cgroup
>> is not provided by this memory device. But when we online memory9, the memory
>> stored page cgroup may be provided by memory8. So we can't offline memory8
>> now. We should offline the memory in the reversed order.
>>
>> When the memory device is hotremoved, we will auto offline memory provided
>> by this memory device. But we don't know which memory is onlined first, so
>> offlining memory may fail. In such case, iterate twice to offline the memory.
>> 1st iterate: offline every non primary memory block.
>> 2nd iterate: offline primary (i.e. first added) memory block.
>>
>> This idea is suggested by KOSAKI Motohiro.
>>
>> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
> 
> Maybe there is something here that I am missing - I admit that I came
> late to this one, but this really sounds like a very ugly hack, that
> really has no place in here.
> 
> Retrying, of course, may make sense, if we have reasonable belief that
> we may now succeed. If this is the case, you need to document - in the
> code - while is that.
> 
> The memcg argument, however, doesn't really cut it. Why can't we make
> all page_cgroup allocations local to the node they are describing? If
> memcg is the culprit here, we should fix it, and not retry. If there is
> still any benefit in retrying, then we retry being very specific about why.

We try to make all page_cgroup allocations local to the node they are describing
now. If the memory is the first memory onlined in this node, we will allocate
it from the other node.

For example, node1 has 4 memory blocks: 8-11, and we online it from 8 to 11
1. memory block 8, page_cgroup allocations are in the other nodes
2. memory block 9, page_cgroup allocations are in memory block 8

So we should offline memory block 9 first. But we don't know in which order
the user online the memory block.

I think we can modify memcg like this:
allocate the memory from the memory block they are describing

I am not sure it is OK to do so.

Thanks
Wen Congyang

> 
> 
> 

^ permalink raw reply

* Re: [PATCH v5 14/14] memory-hotplug: free node_data when a node is offlined
From: Wen Congyang @ 2012-12-30  6:02 UTC (permalink / raw)
  To: Kamezawa Hiroyuki
  Cc: linux-ia64, linux-sh, Tang Chen, linux-mm, paulus, hpa,
	sparclinux, cl, linux-s390, x86, linux-acpi, isimatu.yasuaki,
	linfeng, mgorman, kosaki.motohiro, rientjes, liuj97, len.brown,
	cmetcalf, wujianguo, yinghai, laijs, linux-kernel, minchan.kim,
	akpm, linuxppc-dev
In-Reply-To: <50DCE7C0.8070407@jp.fujitsu.com>

At 12/28/2012 08:28 AM, Kamezawa Hiroyuki Wrote:
> (2012/12/27 21:16), Wen Congyang wrote:
>> At 12/26/2012 11:55 AM, Kamezawa Hiroyuki Wrote:
>>> (2012/12/24 21:09), Tang Chen wrote:
>>>> From: Wen Congyang <wency@cn.fujitsu.com>
>>>>
>>>> We call hotadd_new_pgdat() to allocate memory to store node_data. So we
>>>> should free it when removing a node.
>>>>
>>>> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
>>>
>>> I'm sorry but is it safe to remove pgdat ? All zone cache and zonelists are
>>> properly cleared/rebuilded in synchronous way ? and No threads are visinting
>>> zone in vmscan.c ?
>>
>> We have rebuilt zonelists when a zone has no memory after offlining some pages.
>>
> 
> How do you guarantee that the address of pgdat/zone is not on stack of any kernel
> threads or other kernel objects without reference counting or other syncing method ?

No way to guarentee this. But, the kernel should not use the address of pgdat/zone when
it is offlined.

Hmm, what about this: reuse the memory when the node is onlined again?

Thanks
Wen Congyang

> 
> 
> Thanks,
> -Kame
> 
> 
> 

^ permalink raw reply

* [PATCH] Added device tree binding for TDM and TDM phy
From: Sandeep Singh @ 2013-01-02 13:25 UTC (permalink / raw)
  To: devicetree-discuss, linuxppc-dev; +Cc: Sandeep Singh, Poonam Aggrwal

This controller is available on many Freescale SOCs like MPC8315, P1020, P1010
and P1022

Signed-off-by: Sandeep Singh <Sandeep@freescale.com>
Signed-off-by: Poonam Aggrwal <poonam.aggrwal@freescale.com>
---
 .../devicetree/bindings/powerpc/fsl/fsl-tdm.txt    |   63 ++++++++++++++++++++
 .../devicetree/bindings/powerpc/fsl/tdm-phy.txt    |   38 ++++++++++++
 2 files changed, 101 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/powerpc/fsl/fsl-tdm.txt
 create mode 100644 Documentation/devicetree/bindings/powerpc/fsl/tdm-phy.txt

diff --git a/Documentation/devicetree/bindings/powerpc/fsl/fsl-tdm.txt b/Documentation/devicetree/bindings/powerpc/fsl/fsl-tdm.txt
new file mode 100644
index 0000000..ceb2ef1
--- /dev/null
+++ b/Documentation/devicetree/bindings/powerpc/fsl/fsl-tdm.txt
@@ -0,0 +1,63 @@
+TDM Device Tree Binding
+
+NOTE: The bindings described in this document are preliminary
+and subject to change.
+
+TDM (Time Division Multiplexing)
+
+Description:
+
+The TDM is full duplex serial port designed to allow various devices including
+digital signal processors (DSPs) to communicate with a variety of serial devices
+including industry standard framers, codecs, other DSPs and microprocessors.
+
+The below properties describe the device tree bindings for Freescale TDM
+controller. This TDM controller is available on various Freescale Processors
+like MPC8315, P1020, P1022 and P1010.
+
+Required properties:
+
+- compatible
+    Value type: <string>
+    Definition: Should contain "fsl,tdm1.0".
+
+- reg
+    Definition: A standard property. The first reg specifier describes the TDM
+    registers, and the second describes the TDM DMAC registers.
+
+- tdm_tx_clk
+    Value type: <u32 or u64>
+    Definition: This specifies the value of transmit clock. It should not
+    exceed 50Mhz.
+
+- tdm_rx_clk
+    Value type: <u32 or u64>
+    Definition: This specifies the value of receive clock. Its value could be
+    zero, in which case tdm will operate in shared mode. Its value should not
+    exceed 50Mhz.
+
+- interrupts
+    Definition: Two interrupt specifiers. The first is TDM error, and the
+    second is TDM DMAC.
+
+- phy-handle
+    Value type: <phandle>
+    Definition: Phandle of the line controller node or framer node eg. SLIC,
+    E1/T1 etc. (Refer Documentation/devicetree/bindings/powerpc/fsl/tdm-phy.txt)
+
+- fsl,max-time-slots
+    Value type: <u32>
+    Definition: Maximum number of 8-bit time slots in one TDM frame. This is
+    the maximum number which TDM hardware supports.
+
+Example:
+
+	tdm@16000 {
+		compatible = "fsl,tdm1.0";
+		reg = <0x16000 0x200 0x2c000 0x2000>;
+		tdm_tx_clk = <2048000>;
+		tdm_rx_clk = <0>;
+		interrupts = <16 8 62 8>;
+		phy-handle = <&tdm-phy>;
+		fsl,max-time-slots = <128>;
+	};
diff --git a/Documentation/devicetree/bindings/powerpc/fsl/tdm-phy.txt b/Documentation/devicetree/bindings/powerpc/fsl/tdm-phy.txt
new file mode 100644
index 0000000..2563934
--- /dev/null
+++ b/Documentation/devicetree/bindings/powerpc/fsl/tdm-phy.txt
@@ -0,0 +1,38 @@
+TDM PHY Device Tree Binding
+
+NOTE: The bindings described in this document are preliminary
+and subject to change.
+
+Description:
+TDM PHY is the terminal interface of TDM subsystem. It is typically a line
+control device like E1/T1 framer or SLIC. A TDM device can have multiple TDM
+PHYs.
+
+Required properties:
+
+- compatible
+    Value type: <string>
+    Definition: Should contain generic compatibility like "tdm-phy-slic" or
+    "tdm-phy-e1" or "tdm-phy-t1".
+
+- max-num-ports
+    Value type: <u32>
+    Definition: Defines the maximum number of ports supported by the SLIC
+    device. Only required if the device is SLIC. For E1/T1 devices the number
+    of ports are predefined i.e. (24 in case of T1 and 32 in case of E1).
+
+Apart from the above, there may be other properties required because of the
+bus/interface this device is connected on. It could be SPI/local bus, etc.
+
+Example:
+
+	tdm-phy@0 {
+		compatible = "zarlink,le88266","tdm-phy-slic";
+		reg = <0>;
+		max-num-ports = <4>;
+		spi-max-frequency = <8000000>;
+	};
+
+In the above example properties "reg" and "spi-max-frequency" are SPI specific
+as the SLIC device is connected on SPI interface. These properties might vary
+depending on the specific interface the device is using.
-- 
1.7.6.GIT

^ permalink raw reply related

* Re: [PATCH v5 04/14] memory-hotplug: remove /sys/firmware/memmap/X sysfs
From: Christoph Lameter @ 2013-01-02 14:24 UTC (permalink / raw)
  To: Tang Chen
  Cc: linux-ia64, linux-sh, linux-mm, paulus, hpa, sparclinux,
	linux-s390, x86, linux-acpi, isimatu.yasuaki, linfeng, mgorman,
	kosaki.motohiro, rientjes, liuj97, len.brown, wency, cmetcalf,
	wujianguo, yinghai, Kamezawa Hiroyuki, laijs, linux-kernel,
	minchan.kim, akpm, linuxppc-dev
In-Reply-To: <50DBBBEF.70701@cn.fujitsu.com>

On Thu, 27 Dec 2012, Tang Chen wrote:

> On 12/26/2012 11:30 AM, Kamezawa Hiroyuki wrote:
> >> @@ -41,6 +42,7 @@ struct firmware_map_entry {
> >>    	const char		*type;	/* type of the memory range */
> >>    	struct list_head	list;	/* entry for the linked list */
> >>    	struct kobject		kobj;   /* kobject for each entry */
> >> +	unsigned int		bootmem:1; /* allocated from bootmem */
> >>    };
> >
> > Can't we detect from which the object is allocated from, slab or bootmem ?
> >
> > Hm, for example,
> >
> >      PageReserved(virt_to_page(address_of_obj)) ?
> >      PageSlab(virt_to_page(address_of_obj)) ?
> >
>
> Hi Kamezawa-san,
>
> I think we can detect it without a new member. I think bootmem:1 member
> is just for convenience. I think I can remove it. :)

Larger size slab allocations may fall back to the page allocator but then
the slabs do not track this allocation. That memory can be freed using the
page allocator.

If you see pageslab then you can always remove using the slab allocator.
Otherwise the page allocator should work (unless it was some
special case bootmem allocation).

^ permalink raw reply

* Re: [PATCH 2/5] perf: Make EVENT_ATTR and EVENT_PTR global
From: Jiri Olsa @ 2013-01-02 14:58 UTC (permalink / raw)
  To: Sukadev Bhattiprolu
  Cc: Andi Kleen, Peter Zijlstra, robert.richter, Anton Blanchard,
	linux-kernel, Stephane Eranian, linuxppc-dev, Ingo Molnar,
	Paul Mackerras, Arnaldo Carvalho de Melo
In-Reply-To: <20121219072802.GA30790@us.ibm.com>

On Tue, Dec 18, 2012 at 11:28:02PM -0800, Sukadev Bhattiprolu wrote:
> 
> Rename EVENT_ATTR() and EVENT_PTR() PMU_EVENT_ATTR() and PMU_EVENT_PTR().
> Make them global so they are available to all architectures.
> 
> Further to allow architectures flexibility, have PMU_EVENT_PTR() pass in the
> variable name as a parameter.
> 
hi,
the change looks ok apart from some nits below.

There' another version of the x86 event attributes change
I mentioned earlier:

http://marc.info/?l=linux-kernel&m=135601815224373&w=2

I'm not sure which one will make it in first, but you
guys need to sync ;-) CC-ing Andi and Stephane.

thanks,
jirka

> Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
> ---
>  arch/x86/kernel/cpu/perf_event.c |   17 +++++------------
>  include/linux/perf_event.h       |   13 +++++++++++++
>  2 files changed, 18 insertions(+), 12 deletions(-)
> 
> diff --git a/arch/x86/kernel/cpu/perf_event.c b/arch/x86/kernel/cpu/perf_event.c
> index 4428fd1..24bc505 100644
> --- a/arch/x86/kernel/cpu/perf_event.c
> +++ b/arch/x86/kernel/cpu/perf_event.c
> @@ -1316,11 +1316,6 @@ static struct attribute_group x86_pmu_format_group = {
>  	.attrs = NULL,
>  };
>  
> -struct perf_pmu_events_attr {
> -	struct device_attribute attr;
> -	u64 id;
> -};
> -
>  /*
>   * Remove all undefined events (x86_pmu.event_map(id) == 0)
>   * out of events_attr attributes.
> @@ -1351,14 +1346,12 @@ static ssize_t events_sysfs_show(struct device *dev, struct device_attribute *at
>  	return x86_pmu.events_sysfs_show(page, config);
>  }
>  
> -#define EVENT_VAR(_id)  event_attr_##_id
> -#define EVENT_PTR(_id) &event_attr_##_id.attr.attr
> +#define EVENT_VAR(_id)	event_attr_##_id
> +#define EVENT_ID(_id)	PERF_COUNT_HW_##_id
> +#define EVENT_PTR(_id)	PMU_EVENT_PTR(EVENT_VAR(_id))
>  
> -#define EVENT_ATTR(_name, _id)					\
> -static struct perf_pmu_events_attr EVENT_VAR(_id) = {		\
> -	.attr = __ATTR(_name, 0444, events_sysfs_show, NULL),	\
> -	.id   =  PERF_COUNT_HW_##_id,				\
> -};
> +#define EVENT_ATTR(_name, _id)						\
> +	PMU_EVENT_ATTR(_name, EVENT_VAR(_id), EVENT_ID(_id), events_sysfs_show)

probably no need to define EVENT_ID macro if it's used on just one place

>  
>  EVENT_ATTR(cpu-cycles,			CPU_CYCLES		);
>  EVENT_ATTR(instructions,		INSTRUCTIONS		);
> diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
> index 6bfb2fa..31692cb 100644
> --- a/include/linux/perf_event.h
> +++ b/include/linux/perf_event.h
> @@ -817,6 +817,19 @@ do {									\
>  } while (0)
>  
>  
> +struct perf_pmu_events_attr {
> +	struct device_attribute attr;
> +	u64 id;
> +};
> +
> +#define PMU_EVENT_PTR(_var)	&_var.attr.attr

this one seems superfluous as well, could be replaced by '&'

^ permalink raw reply

* RE: [PATCH 0/4] iommu/fsl: Freescale PAMU driver and IOMMU API implementation.
From: Sethi Varun-B16395 @ 2013-01-03  5:21 UTC (permalink / raw)
  To: Joerg Roedel
  Cc: Wood Scott-B07421, joerg.roedel@amd.com, Tabi Timur-B04825,
	linux-kernel@vger.kernel.org, iommu@lists.linux-foundation.org,
	linuxppc-dev@lists.ozlabs.org
In-Reply-To: <1355493114-21776-1-git-send-email-Varun.Sethi@freescale.com>

Hi Joerg,
It's been a while since I submitted this patch. I have tried to address you=
r comments regarding the subwindow attribute. I would really appreciate if =
I can get some feedback on this patch.

Regards
Varun

> -----Original Message-----
> From: Sethi Varun-B16395
> Sent: Friday, December 21, 2012 7:17 AM
> To: 'Joerg Roedel'
> Cc: Sethi Varun-B16395; joerg.roedel@amd.com; iommu@lists.linux-
> foundation.org; linuxppc-dev@lists.ozlabs.org; linux-
> kernel@vger.kernel.org; Tabi Timur-B04825; Wood Scott-B07421
> Subject: RE: [PATCH 0/4] iommu/fsl: Freescale PAMU driver and IOMMU API
> implementation.
>=20
> ping!!
>=20
> > -----Original Message-----
> > From: Sethi Varun-B16395
> > Sent: Friday, December 14, 2012 7:22 PM
> > To: joerg.roedel@amd.com; iommu@lists.linux-foundation.org; linuxppc-
> > dev@lists.ozlabs.org; linux-kernel@vger.kernel.org; Tabi Timur-B04825;
> > Wood Scott-B07421
> > Cc: Sethi Varun-B16395
> > Subject: [PATCH 0/4] iommu/fsl: Freescale PAMU driver and IOMMU API
> > implementation.
> >
> > This patchset provides the Freescale PAMU (Peripheral Access
> > Management
> > Unit) driver and the corresponding IOMMU API implementation. PAMU is
> > the IOMMU present on Freescale QorIQ platforms. PAMU can authorize
> > memory access, remap the memory address, and remap the I/O transaction
> type.
> >
> > This set consists of the following patches:
> > 1. Addition of new field in the device (powerpc) archdata structure
> > for storing iommu domain information
> >    pointer. This pointer is stored when the device is attached to a
> > particular iommu domain.
> > 2. Add PAMU bypass enable register to the ccsr_guts structure.
> > 3. Addition of domain attributes required by the PAMU driver IOMMU API.
> > 4. PAMU driver and IOMMU API implementation.
> >
> > This patch set is based on the next branch of the iommu git tree
> > maintained by Joerg.
> >
> > Varun Sethi (4):
> >   store iommu domain info in device arch data.
> >   add pamu bypass enable register to guts.
> >   Add iommu attributes for PAMU
> >   FSL PAMU driver.
> >
> >  arch/powerpc/include/asm/device.h   |    4 +
> >  arch/powerpc/include/asm/fsl_guts.h |    4 +-
> >  drivers/iommu/Kconfig               |    8 +
> >  drivers/iommu/Makefile              |    1 +
> >  drivers/iommu/fsl_pamu.c            | 1152
> > +++++++++++++++++++++++++++++++++++
> >  drivers/iommu/fsl_pamu.h            |  398 ++++++++++++
> >  drivers/iommu/fsl_pamu_domain.c     | 1033
> > +++++++++++++++++++++++++++++++
> >  drivers/iommu/fsl_pamu_domain.h     |   96 +++
> >  include/linux/iommu.h               |   49 ++
> >  9 files changed, 2744 insertions(+), 1 deletions(-)  create mode
> > 100644 drivers/iommu/fsl_pamu.c  create mode 100644
> > drivers/iommu/fsl_pamu.h create mode 100644
> > drivers/iommu/fsl_pamu_domain.c  create mode 100644
> > drivers/iommu/fsl_pamu_domain.h
> >
> > --
> > 1.7.4.1

^ permalink raw reply

* [git pull] Please pull powerpc.git merge branch
From: Benjamin Herrenschmidt @ 2013-01-03  6:13 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: linuxppc-dev list, Linux Kernel list

Hi Linus !

Here are a couple of small powerpc fixes. They aren't new bugs (and
they are both CCed to stable) but I didn't see the point of sitting
on the fixes any longer.

Oh and happy new year !

Cheers,
Ben.

The following changes since commit d1c3ed669a2d452cacfb48c2d171a1f364dae2ed:

  Linux 3.8-rc2 (2013-01-02 18:13:21 -0800)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/benh/powerpc.git 

for you to fetch changes up to e6449c9b2d90c1bd9a5985bf05ddebfd1631cd6b:

  powerpc: Add missing NULL terminator to avoid boot panic on PPC40x (2013-01-03 16:45:52 +1100)

----------------------------------------------------------------
Gabor Juhos (1):
      powerpc: Add missing NULL terminator to avoid boot panic on PPC40x

Shan Hai (1):
      powerpc/vdso: Remove redundant locking in update_vsyscall_tz()

 arch/powerpc/kernel/time.c                 |    5 -----
 arch/powerpc/platforms/40x/ppc40x_simple.c |    3 ++-
 2 files changed, 2 insertions(+), 6 deletions(-)

^ permalink raw reply

* Re: [git pull] Please pull powerpc.git merge branch
From: Benjamin Herrenschmidt @ 2013-01-03  6:14 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: linuxppc-dev list, Linux Kernel list
In-Reply-To: <1357193608.3924.37.camel@pasglop>

On Thu, 2013-01-03 at 17:13 +1100, Benjamin Herrenschmidt wrote:
> Hi Linus !
> 
> Here are a couple of small powerpc fixes. They aren't new bugs (and
> they are both CCed to stable) but I didn't see the point of sitting
> on the fixes any longer.

Looks like I still need to fix my script to get the branch name right
for when the mirrors haven't caught up...

It's

  git://git.kernel.org/pub/scm/linux/kernel/git/benh/powerpc.git merge

Cheers,
Ben.

^ permalink raw reply

* tqm5200s i2c bus timeout
From: Johannes Braun @ 2013-01-03 13:20 UTC (permalink / raw)
  To: linuxppc-dev

Hello,

I hope someone could help me with my problem. Currently I am porting
a new kernel (3.3.8) for a tqm5200s based board.
The previous kernel was 2.6.23. The new kernel version is needed because
of support for a wireless card.

I got issues with the i2c bus and this kernel. When the kernel boots up,
the i2c initialization ends in a timout. This is the kernel log:

[    1.460652] i2c /dev entries driver
[    1.465434] mpc-i2c f0003d40.i2c: timeout 1000000 us

Connected to the bus is an eeprom (Microchip 24c32a) and a realtime
clock (Philips PCF8563).
The i2c bus section in the dtb file looks as follows:

i2c@3d40 {
     #address-cells = <1>;
     #size-cells = <0>;
     compatible = "fsl,mpc5200-i2c","fsl-i2c";
     reg = <0x3d40 0x40>;
     interrupts = <2 16 0>;
     fsl5200-clocking;
};

The dtb file from the kernel 2.6.23 looks as follows:
i2c@3d40 {

#address-cells = <1>;
     #size-cells = <0>;
     compatible = "fsl,mpc5200-i2c","fsl-i2c";
     reg = <0x3d40 0x40>;
     interrupts = <2 16 0>;
     fsl5200-clocking;
};

I can`t see any devices in /sys/bus/i2c/devices except the bus itself.
# ls /sys/bus/i2c/devices
# i2c-0
# cat /sys/bus/i2c/devices/i2c-0/name
# MPC adapter

Is there something wrong with my dtb file or is it a bug in the mpc-i2c driver

Best regards
Johannes

^ permalink raw reply

* Re: tqm5200s i2c bus timeout
From: Johannes Braun @ 2013-01-03 13:25 UTC (permalink / raw)
  To: linuxppc-dev
In-Reply-To: <CADwvPCve5GaRZZXuLUq4Es2FLN8AxtbRP4TQP7gQVRXoMw3S0g@mail.gmail.com>

> The dtb file from the kernel 2.6.23 looks as follows:
> i2c@3d40 {
>
> #address-cells = <1>;
>      #size-cells = <0>;
>      compatible = "fsl,mpc5200-i2c","fsl-i2c";
>      reg = <0x3d40 0x40>;
>      interrupts = <2 16 0>;
>      fsl5200-clocking;
> };

sry, but this was a mistake. This is the correct section from the kernel 2.6.23:

i2c@3d40 {
    device_type = "i2c";
    compatible = "mpc5200-i2c\0fsl-i2c";
    cell-index= <1>;
    reg = <3d40 40>;
    interrupts = <2 10 0>;
    interrupt-parent = <&mpc5200_pic>;
    fsl5200-clocking;
};

^ permalink raw reply

* Re: tqm5200s i2c bus timeout
From: Anatolij Gustschin @ 2013-01-03 14:11 UTC (permalink / raw)
  To: Johannes Braun; +Cc: linuxppc-dev
In-Reply-To: <CADwvPCve5GaRZZXuLUq4Es2FLN8AxtbRP4TQP7gQVRXoMw3S0g@mail.gmail.com>

Hi,

On Thu, 3 Jan 2013 14:20:41 +0100
Johannes Braun <jjo.braun@gmail.com> wrote:

> Hello,
> 
> I hope someone could help me with my problem. Currently I am porting
> a new kernel (3.3.8) for a tqm5200s based board.
> The previous kernel was 2.6.23. The new kernel version is needed because
> of support for a wireless card.
> 
> I got issues with the i2c bus and this kernel. When the kernel boots up,
> the i2c initialization ends in a timout. This is the kernel log:
> 
> [    1.460652] i2c /dev entries driver
> [    1.465434] mpc-i2c f0003d40.i2c: timeout 1000000 us

No, the initialization doesn't end in timeout. It is just an info
which timeout value will be used for i2c transfers.

> 
> Connected to the bus is an eeprom (Microchip 24c32a) and a realtime
> clock (Philips PCF8563).
> The i2c bus section in the dtb file looks as follows:
> 
> i2c@3d40 {
>      #address-cells = <1>;
>      #size-cells = <0>;
>      compatible = "fsl,mpc5200-i2c","fsl-i2c";
>      reg = <0x3d40 0x40>;
>      interrupts = <2 16 0>;
>      fsl5200-clocking;

This "fsl5200-clocking" property is not needed anymore, you can remove it.

> };

> 
> The dtb file from the kernel 2.6.23 looks as follows:
> i2c@3d40 {
> 
> #address-cells = <1>;
>      #size-cells = <0>;
>      compatible = "fsl,mpc5200-i2c","fsl-i2c";
>      reg = <0x3d40 0x40>;
>      interrupts = <2 16 0>;
>      fsl5200-clocking;
> };
> 
> I can`t see any devices in /sys/bus/i2c/devices except the bus itself.
> # ls /sys/bus/i2c/devices
> # i2c-0
> # cat /sys/bus/i2c/devices/i2c-0/name
> # MPC adapter

This is expected since you didn't add sub-nodes for your i2c devices
24c32a and PCF8563 in the i2c adapter node.

> Is there something wrong with my dtb file or is it a bug in the mpc-i2c driver

It is an issue with our dtb file. Please look at the I2C eeprom
sub-node in the arch/powerpc/boot/dts/mpc5121ads.dts file and at
the pcf8563 RTC sub-node in the arch/powerpc/boot/dts/mucmc52.dts
file for an example how to add needed nodes for your devices.

Thanks,
Anatolij

^ permalink raw reply


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