Re: [PATCH] powerpc: Add second POWER8 PVR entry
Anshuman Khandual wrote: > On 07/18/2013 07:01 AM, Michael Neuling wrote: > > POWER8 comes with two different PVRs. This patch enables the additional > > PVR in the cputable. > > > > The existing entry (PVR=0x4b) is renamed to POWER8E and the new entry > > (PVR=0x4d) is given POWER8. > > Hey Mikey, > > Is there any feature or architectural difference between these two different > PVR based P8 chips ? I've kept the CPU_FTRs the same. Mikey ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] powerpc: Add second POWER8 PVR entry
On 07/18/2013 07:01 AM, Michael Neuling wrote: > POWER8 comes with two different PVRs. This patch enables the additional > PVR in the cputable. > > The existing entry (PVR=0x4b) is renamed to POWER8E and the new entry > (PVR=0x4d) is given POWER8. Hey Mikey, Is there any feature or architectural difference between these two different PVR based P8 chips ? Regards Anshuman ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] module: ppc64 module CRC relocation fix causes perf issues
Hi Scott, > What specifically should I do to test it? Could you double check perf annotate works? I'm 99% sure it will but that is what was failing on ppc64. Anton ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 3/4 V2] mmc: esdhc: Correct host version of T4240-R1.0
> -Original Message- > From: linux-mmc-ow...@vger.kernel.org [mailto:linux-mmc- > ow...@vger.kernel.org] On Behalf Of Scott Wood > > On 07/17/2013 05:11:30 AM, Haijun Zhang wrote: > > Vender version and sdhc spec version of T4240-R1.0 is incorrect. > > The right value should be VVN=0x13, SVN = 0x1. The wrong version > > number will break down the ADMA data transfer. > > This defect only exist in T4240-R1.0. Will be fixed in T4240-R2.0. > > Also share vvn and svr for public use. > > > > Signed-off-by: Haijun Zhang > > We're not supporting T4240 rev 1.0 in upstream Linux, as it's not > production silicon. As discussed, production silicon Rev 2.0 will also suffer this issue. So need to address it beyond T4240-R2.0. thanks. Roy ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 10/10] powerpc/eeh: Fix unbalanced enable for IRQ
The patch fixes following issue: Unbalanced enable for IRQ 23 [ cut here ] WARNING: at kernel/irq/manage.c:437 : NIP [c016de8c] .__enable_irq+0x11c/0x140 LR [c016de88] .__enable_irq+0x118/0x140 Call Trace: [c03ea1f23880] [c016de88] .__enable_irq+0x118/0x140 (unreliable) [c03ea1f23910] [c016df08] .enable_irq+0x58/0xa0 [c03ea1f239a0] [c00388b4] .eeh_enable_irq+0xc4/0xe0 [c03ea1f23a30] [c0038a28] .eeh_report_reset+0x78/0x130 [c03ea1f23ac0] [c0037508] .eeh_pe_dev_traverse+0x98/0x170 [c03ea1f23b60] [c00391ac] .eeh_handle_normal_event+0x2fc/0x3d0 [c03ea1f23bf0] [c0039538] .eeh_handle_event+0x2b8/0x2c0 [c03ea1f23c90] [c0039600] .eeh_event_handler+0xc0/0x170 [c03ea1f23d30] [c00da9a0] .kthread+0xf0/0x100 [c03ea1f23e30] [c000a1dc] .ret_from_kernel_thread+0x5c/0x80 Signed-off-by: Gavin Shan --- arch/powerpc/kernel/eeh_driver.c |6 +- 1 files changed, 5 insertions(+), 1 deletions(-) diff --git a/arch/powerpc/kernel/eeh_driver.c b/arch/powerpc/kernel/eeh_driver.c index 3fee021..72c4a17 100644 --- a/arch/powerpc/kernel/eeh_driver.c +++ b/arch/powerpc/kernel/eeh_driver.c @@ -143,10 +143,14 @@ static void eeh_disable_irq(struct pci_dev *dev) static void eeh_enable_irq(struct pci_dev *dev) { struct eeh_dev *edev = pci_dev_to_eeh_dev(dev); + struct irq_desc *desc; if ((edev->mode) & EEH_DEV_IRQ_DISABLED) { edev->mode &= ~EEH_DEV_IRQ_DISABLED; - enable_irq(dev->irq); + + desc = irq_to_desc(dev->irq); + if (desc && desc->depth > 0) + enable_irq(dev->irq); } } -- 1.7.5.4 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 09/10] powerpc/eeh: Don't use pci_dev during BAR restore
While restoring BARs for one specific PCI device, the pci_dev instance should have been released. So it's not reliable to use the pci_dev instance on restoring BARs. However, we still need some information (e.g. PCIe capability position, header type) from the pci_dev instance. So we have to store those information to EEH device in advance. Signed-off-by: Gavin Shan --- arch/powerpc/include/asm/eeh.h |8 +++- arch/powerpc/kernel/eeh_pe.c | 25 +- arch/powerpc/platforms/powernv/eeh-powernv.c | 11 + arch/powerpc/platforms/pseries/eeh_pseries.c | 63 +- 4 files changed, 91 insertions(+), 16 deletions(-) diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h index f54a601..4199d99 100644 --- a/arch/powerpc/include/asm/eeh.h +++ b/arch/powerpc/include/asm/eeh.h @@ -84,8 +84,11 @@ struct eeh_pe { * another tree except the currently existing tree of PCI * buses and PCI devices */ -#define EEH_DEV_IRQ_DISABLED (1 << 0)/* Interrupt disabled */ -#define EEH_DEV_DISCONNECTED (1 << 1)/* Removing from PE */ +#define EEH_DEV_BRIDGE (1 << 0)/* PCI bridge */ +#define EEH_DEV_ROOT_PORT (1 << 1)/* PCIe root port */ +#define EEH_DEV_DS_PORT(1 << 2)/* Downstream port */ +#define EEH_DEV_IRQ_DISABLED (1 << 3)/* Interrupt disabled */ +#define EEH_DEV_DISCONNECTED (1 << 4)/* Removing from PE */ struct eeh_dev { int mode; /* EEH mode */ @@ -93,6 +96,7 @@ struct eeh_dev { int config_addr;/* Config address */ int pe_config_addr; /* PE config address*/ u32 config_space[16]; /* Saved PCI config space */ + u8 pcie_cap;/* Saved PCIe capability*/ struct eeh_pe *pe; /* Associated PE*/ struct list_head list; /* Form link list in the PE */ struct pci_controller *phb; /* Associated PHB */ diff --git a/arch/powerpc/kernel/eeh_pe.c b/arch/powerpc/kernel/eeh_pe.c index 2aa955a..f945053 100644 --- a/arch/powerpc/kernel/eeh_pe.c +++ b/arch/powerpc/kernel/eeh_pe.c @@ -578,7 +578,7 @@ void eeh_pe_state_clear(struct eeh_pe *pe, int state) * blocked on normal path during the stage. So we need utilize * eeh operations, which is always permitted. */ -static void eeh_bridge_check_link(struct pci_dev *pdev, +static void eeh_bridge_check_link(struct eeh_dev *edev, struct device_node *dn) { int cap; @@ -589,16 +589,17 @@ static void eeh_bridge_check_link(struct pci_dev *pdev, * We only check root port and downstream ports of * PCIe switches */ - if (!pci_is_pcie(pdev) || - (pci_pcie_type(pdev) != PCI_EXP_TYPE_ROOT_PORT && -pci_pcie_type(pdev) != PCI_EXP_TYPE_DOWNSTREAM)) + if (!(edev->mode & (EEH_DEV_ROOT_PORT | EEH_DEV_DS_PORT))) return; - pr_debug("%s: Check PCIe link for %s ...\n", -__func__, pci_name(pdev)); + pr_debug("%s: Check PCIe link for %04x:%02x:%02x.%01x ...\n", +__func__, edev->phb->global_number, +edev->config_addr >> 8, +PCI_SLOT(edev->config_addr & 0xFF), +PCI_FUNC(edev->config_addr & 0xFF)); /* Check slot status */ - cap = pdev->pcie_cap; + cap = edev->pcie_cap; eeh_ops->read_config(dn, cap + PCI_EXP_SLTSTA, 2, &val); if (!(val & PCI_EXP_SLTSTA_PDS)) { pr_debug(" No card in the slot (0x%04x) !\n", val); @@ -652,8 +653,7 @@ static void eeh_bridge_check_link(struct pci_dev *pdev, #define BYTE_SWAP(OFF) (8*((OFF)/4)+3-(OFF)) #define SAVED_BYTE(OFF)(((u8 *)(edev->config_space))[BYTE_SWAP(OFF)]) -static void eeh_restore_bridge_bars(struct pci_dev *pdev, - struct eeh_dev *edev, +static void eeh_restore_bridge_bars(struct eeh_dev *edev, struct device_node *dn) { int i; @@ -679,7 +679,7 @@ static void eeh_restore_bridge_bars(struct pci_dev *pdev, eeh_ops->write_config(dn, PCI_COMMAND, 4, edev->config_space[1]); /* Check the PCIe link is ready */ - eeh_bridge_check_link(pdev, dn); + eeh_bridge_check_link(edev, dn); } static void eeh_restore_device_bars(struct eeh_dev *edev, @@ -729,12 +729,11 @@ static void eeh_restore_device_bars(struct eeh_dev *edev, static void *eeh_restore_one_device_bars(void *data, void *flag) { struct eeh_dev *edev = (struct eeh_dev *)data; - struct pci_dev *pdev = eeh_dev_to_pci_dev(edev); struct device_node *dn = eeh_dev_to_of_node(edev); /* Do special restore for bridges */ - if (pdev->hdr_type ==
[PATCH 08/10] powerpc/eeh: Support partial hotplug
When EEH error happens to one specific PE, some devices with drivers supporting EEH won't except hotplug on the deivce. However, there might have other deivces without driver, or with driver without EEH support. For the case, we need do partial hotplug in order to make sure that the PE becomes absolutely quite during reset. Otherise, the PE reset might fail and leads to failure of error recovery. The patch intends to support so-called "partial" hotplug for EEH: Before we do reset, we stop and remove those PCI devices without EEH sensitive driver. The corresponding EEH devices are not detached from its PE, but with special flag. After the reset is done, those EEH devices with the special flag will be scanned one by one. Signed-off-by: Gavin Shan --- arch/powerpc/include/asm/eeh.h |6 ++- arch/powerpc/kernel/eeh.c| 30 ++- arch/powerpc/kernel/eeh_driver.c | 106 -- arch/powerpc/kernel/eeh_pe.c | 20 +++- arch/powerpc/kernel/eeh_sysfs.c |7 +++ 5 files changed, 147 insertions(+), 22 deletions(-) diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h index e8c411b..f54a601 100644 --- a/arch/powerpc/include/asm/eeh.h +++ b/arch/powerpc/include/asm/eeh.h @@ -84,7 +84,8 @@ struct eeh_pe { * another tree except the currently existing tree of PCI * buses and PCI devices */ -#define EEH_DEV_IRQ_DISABLED (1<<0) /* Interrupt disabled */ +#define EEH_DEV_IRQ_DISABLED (1 << 0)/* Interrupt disabled */ +#define EEH_DEV_DISCONNECTED (1 << 1)/* Removing from PE */ struct eeh_dev { int mode; /* EEH mode */ @@ -97,6 +98,7 @@ struct eeh_dev { struct pci_controller *phb; /* Associated PHB */ struct device_node *dn; /* Associated device node */ struct pci_dev *pdev; /* Associated PCI device*/ + struct pci_bus *bus;/* PCI bus for partial hotplug */ }; static inline struct device_node *eeh_dev_to_of_node(struct eeh_dev *edev) @@ -197,6 +199,8 @@ struct eeh_pe *eeh_pe_get(struct eeh_dev *edev); int eeh_add_to_parent_pe(struct eeh_dev *edev); int eeh_rmv_from_parent_pe(struct eeh_dev *edev); void eeh_pe_update_time_stamp(struct eeh_pe *pe); +void *eeh_pe_traverse(struct eeh_pe *root, + eeh_traverse_func fn, void *flag); void *eeh_pe_dev_traverse(struct eeh_pe *root, eeh_traverse_func fn, void *flag); void eeh_pe_restore_bars(struct eeh_pe *pe); diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c index 56bd458..a5783f1 100644 --- a/arch/powerpc/kernel/eeh.c +++ b/arch/powerpc/kernel/eeh.c @@ -900,7 +900,21 @@ void eeh_add_device_late(struct pci_dev *dev) pr_debug("EEH: Already referenced !\n"); return; } - WARN_ON(edev->pdev); + + /* +* The EEH cache might not be removed correctly because of +* unbalanced kref to the device during unplug time, which +* relies on pcibios_release_device(). So we have to remove +* that here explicitly. +*/ + if (edev->pdev) { + eeh_rmv_from_parent_pe(edev); + eeh_addr_cache_rmv_dev(edev->pdev); + eeh_sysfs_remove_device(edev->pdev); + + edev->pdev = NULL; + dev->dev.archdata.edev = NULL; + } edev->pdev = dev; dev->dev.archdata.edev = edev; @@ -982,14 +996,24 @@ void eeh_remove_device(struct pci_dev *dev) /* Unregister the device with the EEH/PCI address search system */ pr_debug("EEH: Removing device %s\n", pci_name(dev)); - if (!edev || !edev->pdev) { + if (!edev || !edev->pdev || !edev->pe) { pr_debug("EEH: Not referenced !\n"); return; } + + /* +* During the hotplug for EEH error recovery, we need the EEH +* device attached to the parent PE in order for BAR restore +* a bit later. So we keep it for BAR restore and remove it +* from the parent PE during the BAR resotre. +*/ edev->pdev = NULL; dev->dev.archdata.edev = NULL; + if (!(edev->pe->state & EEH_PE_KEEP)) + eeh_rmv_from_parent_pe(edev); + else + edev->mode |= EEH_DEV_DISCONNECTED; - eeh_rmv_from_parent_pe(edev); eeh_addr_cache_rmv_dev(dev); eeh_sysfs_remove_device(dev); } diff --git a/arch/powerpc/kernel/eeh_driver.c b/arch/powerpc/kernel/eeh_driver.c index 9ef3bbb..3fee021 100644 --- a/arch/powerpc/kernel/eeh_driver.c +++ b/arch/powerpc/kernel/eeh_driver.c @@ -338,6 +338,89 @@ static void *eeh_report_failure(void *data, void *userdata) return NULL; } +static void *eeh_rmv_device(void *data, void *userdata) +{ + struct pci_driver *driver; + struct eeh_dev *edev = (struct eeh_dev *)data; + stru
[PATCH 05/10] powerpc/eeh: Keep PE during hotplug
When we do normal hotplug, the PE shouldn't be kept. However, we need the PE if the hotplug caused by EEH errors. Since we remove EEH device through the PCI hook pcibios_stop_dev(), the flag "purge_pe" passed to various functions is meaningless. So the patch removes the meaningless flag and introduce new flag "EEH_PE_KEEP" to save the PE while doing hotplug during EEH error recovery. Signed-off-by: Gavin Shan --- arch/powerpc/include/asm/eeh.h| 11 +-- arch/powerpc/include/asm/pci-bridge.h |1 - arch/powerpc/kernel/eeh.c | 28 ++-- arch/powerpc/kernel/eeh_driver.c |7 +-- arch/powerpc/kernel/eeh_pe.c |7 +++ arch/powerpc/kernel/pci-hotplug.c | 26 +- 6 files changed, 20 insertions(+), 60 deletions(-) diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h index d9d35c2..2ce22d7 100644 --- a/arch/powerpc/include/asm/eeh.h +++ b/arch/powerpc/include/asm/eeh.h @@ -55,6 +55,8 @@ struct device_node; #define EEH_PE_RECOVERING (1 << 1)/* Recovering PE*/ #define EEH_PE_PHB_DEAD(1 << 2)/* Dead PHB */ +#define EEH_PE_KEEP(1 << 8)/* Keep PE on hotplug */ + struct eeh_pe { int type; /* PE type: PHB/Bus/Device */ int state; /* PE EEH dependent mode*/ @@ -193,7 +195,7 @@ int eeh_phb_pe_create(struct pci_controller *phb); struct eeh_pe *eeh_phb_pe_get(struct pci_controller *phb); struct eeh_pe *eeh_pe_get(struct eeh_dev *edev); int eeh_add_to_parent_pe(struct eeh_dev *edev); -int eeh_rmv_from_parent_pe(struct eeh_dev *edev, int purge_pe); +int eeh_rmv_from_parent_pe(struct eeh_dev *edev); void eeh_pe_update_time_stamp(struct eeh_pe *pe); void *eeh_pe_dev_traverse(struct eeh_pe *root, eeh_traverse_func fn, void *flag); @@ -214,8 +216,7 @@ void eeh_add_device_tree_early(struct device_node *); void eeh_add_device_late(struct pci_dev *); void eeh_add_device_tree_late(struct pci_bus *); void eeh_add_sysfs_files(struct pci_bus *); -void eeh_remove_device(struct pci_dev *, int); -void eeh_remove_bus_device(struct pci_dev *, int); +void eeh_remove_device(struct pci_dev *); /** * EEH_POSSIBLE_ERROR() -- test for possible MMIO failure. @@ -265,9 +266,7 @@ 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_device(struct pci_dev *dev, int purge_pe) { } - -static inline void eeh_remove_bus_device(struct pci_dev *dev, int purge_pe) { } +static inline void eeh_remove_device(struct pci_dev *dev) { } #define EEH_POSSIBLE_ERROR(val, type) (0) #define EEH_IO_ERROR_VALUE(size) (-1UL) diff --git a/arch/powerpc/include/asm/pci-bridge.h b/arch/powerpc/include/asm/pci-bridge.h index 2c1d8cb..32d0d20 100644 --- a/arch/powerpc/include/asm/pci-bridge.h +++ b/arch/powerpc/include/asm/pci-bridge.h @@ -209,7 +209,6 @@ static inline struct eeh_dev *of_node_to_eeh_dev(struct device_node *dn) extern struct pci_bus *pcibios_find_pci_bus(struct device_node *dn); /** Remove all of the PCI devices under this bus */ -extern void __pcibios_remove_pci_devices(struct pci_bus *bus, int purge_pe); extern void pcibios_remove_pci_devices(struct pci_bus *bus); /** Discover new pci devices under this bus, and add them */ diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c index 582ad1e..ce81477 100644 --- a/arch/powerpc/kernel/eeh.c +++ b/arch/powerpc/kernel/eeh.c @@ -964,7 +964,6 @@ 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 * * This routine should be called when a device is removed from * a running system (e.g. by hotplug or dlpar). It unregisters @@ -972,7 +971,7 @@ EXPORT_SYMBOL_GPL(eeh_add_sysfs_files); * this device will no longer be detected after this call; thus, * i/o errors affecting this slot may leave this device unusable. */ -void eeh_remove_device(struct pci_dev *dev, int purge_pe) +void eeh_remove_device(struct pci_dev *dev) { struct eeh_dev *edev; @@ -990,34 +989,11 @@ void eeh_remove_device(struct pci_dev *dev, int purge_pe) edev->pdev = NULL; dev->dev.archdata.edev = NULL; - eeh_rmv_from_parent_pe(edev, purge_pe); + eeh_rmv_from_parent_pe(edev); eeh_addr_cache_rmv_dev(dev); eeh_sysfs_remove_device(dev); } -/** - * eeh_remove_bus_device - Undo EEH setup for the indicated PCI device - * @dev: PCI device - * @purge_pe: remove the corresponding PE or not - * - * This routine must be called when a device is removed from the - * running system through hotplug or dlpar. The corresponding - * PCI address cache will be removed. - */ -void eeh_remove_bus_device(struct pci_dev *de
[PATCH 06/10] powerpc/eeh: Tranverse EEH devices with safe mode
Currently, we're transversing EEH devices by list_for_each_entry(). That's not safe enough because the EEH devices might be removed from its parent PE while doing iteration. The patch replaces that with list_for_each_entry_safe(). Signed-off-by: Gavin Shan --- arch/powerpc/include/asm/eeh.h |4 ++-- arch/powerpc/kernel/eeh.c |4 ++-- arch/powerpc/kernel/eeh_pe.c | 10 +- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h index 2ce22d7..e8c411b 100644 --- a/arch/powerpc/include/asm/eeh.h +++ b/arch/powerpc/include/asm/eeh.h @@ -74,8 +74,8 @@ struct eeh_pe { struct list_head child; /* Child PEs*/ }; -#define eeh_pe_for_each_dev(pe, edev) \ - list_for_each_entry(edev, &pe->edevs, list) +#define eeh_pe_for_each_dev(pe, edev, tmp) \ + list_for_each_entry_safe(edev, tmp, &pe->edevs, list) /* * The struct is used to trace EEH state for the associated diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c index ce81477..56bd458 100644 --- a/arch/powerpc/kernel/eeh.c +++ b/arch/powerpc/kernel/eeh.c @@ -231,7 +231,7 @@ static size_t eeh_gather_pci_data(struct eeh_dev *edev, char * buf, size_t len) void eeh_slot_error_detail(struct eeh_pe *pe, int severity) { size_t loglen = 0; - struct eeh_dev *edev; + struct eeh_dev *edev, *tmp; bool valid_cfg_log = true; /* @@ -251,7 +251,7 @@ void eeh_slot_error_detail(struct eeh_pe *pe, int severity) eeh_pe_restore_bars(pe); pci_regs_buf[0] = 0; - eeh_pe_for_each_dev(pe, edev) { + eeh_pe_for_each_dev(pe, edev, tmp) { loglen += eeh_gather_pci_data(edev, pci_regs_buf + loglen, EEH_PCI_REGS_LOG_LEN - loglen); } diff --git a/arch/powerpc/kernel/eeh_pe.c b/arch/powerpc/kernel/eeh_pe.c index 32ef409..c8b815e 100644 --- a/arch/powerpc/kernel/eeh_pe.c +++ b/arch/powerpc/kernel/eeh_pe.c @@ -176,7 +176,7 @@ void *eeh_pe_dev_traverse(struct eeh_pe *root, eeh_traverse_func fn, void *flag) { struct eeh_pe *pe; - struct eeh_dev *edev; + struct eeh_dev *edev, *tmp; void *ret; if (!root) { @@ -186,7 +186,7 @@ void *eeh_pe_dev_traverse(struct eeh_pe *root, /* Traverse root PE */ for (pe = root; pe; pe = eeh_pe_next(pe, root)) { - eeh_pe_for_each_dev(pe, edev) { + eeh_pe_for_each_dev(pe, edev, tmp) { ret = fn(edev, flag); if (ret) return ret; @@ -501,7 +501,7 @@ static void *__eeh_pe_state_mark(void *data, void *flag) { struct eeh_pe *pe = (struct eeh_pe *)data; int state = *((int *)flag); - struct eeh_dev *tmp; + struct eeh_dev *edev, *tmp; struct pci_dev *pdev; /* @@ -511,8 +511,8 @@ static void *__eeh_pe_state_mark(void *data, void *flag) * the PCI device driver. */ pe->state |= state; - eeh_pe_for_each_dev(pe, tmp) { - pdev = eeh_dev_to_pci_dev(tmp); + eeh_pe_for_each_dev(pe, edev, tmp) { + pdev = eeh_dev_to_pci_dev(edev); if (pdev) pdev->error_state = pci_channel_io_frozen; } -- 1.7.5.4 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 03/10] powerpc/pci: Override pcibios_release_device()
The patch overrides pcibios_release_device() to release EEH resources (EEH cache, unbinding EEH device) for the indicated PCI device. Signed-off-by: Gavin Shan --- arch/powerpc/kernel/pci-hotplug.c | 11 +++ 1 files changed, 11 insertions(+), 0 deletions(-) diff --git a/arch/powerpc/kernel/pci-hotplug.c b/arch/powerpc/kernel/pci-hotplug.c index 3f60880..3dab2f2 100644 --- a/arch/powerpc/kernel/pci-hotplug.c +++ b/arch/powerpc/kernel/pci-hotplug.c @@ -22,6 +22,17 @@ #include /** + * pcibios_release_device - release PCI device + * @dev: PCI device + * + * The function is called before releasing the indicated PCI device. + */ +void pcibios_release_device(struct pci_dev *dev) +{ + eeh_remove_device(dev, 1); +} + +/** * __pcibios_remove_pci_devices - remove all devices under this bus * @bus: the indicated PCI bus * @purge_pe: destroy the PE on removal of PCI devices -- 1.7.5.4 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 07/10] powerpc/pci: Partial hotplug support
When EEH error happens to one specific PE, the device drivers of its attached EEH devices (PCI devices) are checked to see the further action: reset with complete hotplug, or reset without hotplug. However, that's not enough for those PCI devices whose drivers can't support EEH, or those PCI devices without driver. So we need do so-called "partial hotplug" on basis of PCI devices. In the situation, part of PCI devices of the specific PE are unplugged and plugged again after PE reset. The patch adds functions to support scanning signle PCI device (function) either based on device-tree or hardware for plugging. The existing function pci_stop_and_remove_bus_device() is enough for unplugging. Besides, the patch also fixes the issue that we need reassign the resources if we had flag PCI_REASSIGN_ALL_RSRC. Otherwise, to claim the resources of attached devices of the PCI bus should fail and the newly added devices in "complete" hotplug can't be enabled. Signed-off-by: Gavin Shan --- arch/powerpc/include/asm/pci-bridge.h |2 +- arch/powerpc/include/asm/pci.h|2 + arch/powerpc/kernel/pci-common.c |8 ++- arch/powerpc/kernel/pci-hotplug.c | 92 + arch/powerpc/kernel/pci_of_scan.c | 43 +++ 5 files changed, 132 insertions(+), 15 deletions(-) diff --git a/arch/powerpc/include/asm/pci-bridge.h b/arch/powerpc/include/asm/pci-bridge.h index 32d0d20..070aed3 100644 --- a/arch/powerpc/include/asm/pci-bridge.h +++ b/arch/powerpc/include/asm/pci-bridge.h @@ -213,7 +213,7 @@ extern void pcibios_remove_pci_devices(struct pci_bus *bus); /** Discover new pci devices under this bus, and add them */ extern void pcibios_add_pci_devices(struct pci_bus *bus); - +void pcibios_scan_pci_dev(struct pci_bus *bus, struct device_node *dn); extern void isa_bridge_find_early(struct pci_controller *hose); diff --git a/arch/powerpc/include/asm/pci.h b/arch/powerpc/include/asm/pci.h index 6653f27..28cfc95 100644 --- a/arch/powerpc/include/asm/pci.h +++ b/arch/powerpc/include/asm/pci.h @@ -167,6 +167,8 @@ extern struct pci_dev *of_create_pci_dev(struct device_node *node, struct pci_bus *bus, int devfn); extern void of_scan_pci_bridge(struct pci_dev *dev); +extern struct pci_dev *of_scan_pci_dev(struct pci_bus *bus, + struct device_node *dn); extern void of_scan_bus(struct device_node *node, struct pci_bus *bus); extern void of_rescan_bus(struct device_node *node, struct pci_bus *bus); diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c index f46914a..6f3a1cb 100644 --- a/arch/powerpc/kernel/pci-common.c +++ b/arch/powerpc/kernel/pci-common.c @@ -1460,8 +1460,12 @@ void pcibios_finish_adding_to_bus(struct pci_bus *bus) pci_domain_nr(bus), bus->number); /* Allocate bus and devices resources */ - pcibios_allocate_bus_resources(bus); - pcibios_claim_one_bus(bus); + if (pci_has_flag(PCI_REASSIGN_ALL_RSRC)) { + pci_assign_unassigned_bus_resources(bus); + } else { + pcibios_allocate_bus_resources(bus); + pcibios_claim_one_bus(bus); + } /* Fixup EEH */ eeh_add_device_tree_late(bus); diff --git a/arch/powerpc/kernel/pci-hotplug.c b/arch/powerpc/kernel/pci-hotplug.c index fc0831d..c79105f 100644 --- a/arch/powerpc/kernel/pci-hotplug.c +++ b/arch/powerpc/kernel/pci-hotplug.c @@ -104,3 +104,95 @@ void pcibios_add_pci_devices(struct pci_bus * bus) pcibios_finish_adding_to_bus(bus); } EXPORT_SYMBOL_GPL(pcibios_add_pci_devices); + +static void pcibios_of_scan_dev(struct pci_bus *bus, struct device_node *dn) +{ + struct pci_dev *dev; + int ret; + + dev = of_scan_pci_dev(bus, dn); + if (!dev) + return; + + eeh_add_device_early(dn); + pcibios_add_device(dev); + eeh_add_device_late(dev); + + ret = pci_bus_add_device(dev); + if (ret) { + pr_info("%s: Failed to add PCI dev %s\n", + __func__, pci_name(dev)); + return; + } + + eeh_sysfs_add_device(dev); +} + +static void pcibios_scan_dev(struct pci_bus *bus, struct device_node *dn) +{ + struct pci_dn *pdn = PCI_DN(dn); + struct pci_dev *dev; + struct resource *r; + int i, ret; + + eeh_add_device_early(dn); + dev = pci_scan_single_device(bus, pdn->devfn); + if (!dev) { + pr_warn("%s: Failed to probe %04x:%02x:%2x.%01x\n", + __func__, pci_domain_nr(bus), bus->number, + PCI_SLOT(pdn->devfn), PCI_FUNC(pdn->devfn)); + return; + } + + /* +* If we already requested to reassign resources, the +* start address of individual resources is zero'ed +* during PCI header fixup time. So we need reassign +* the resource f
[PATCH 04/10] PCI/hotplug: Needn't remove EEH cache again
Since pcibios_release_device() called by pci_stop_and_remove_bus_device() has removed the EEH cache, we needn't do that again. Acked-by: Bjorn Helgaas Signed-off-by: Gavin Shan --- drivers/pci/hotplug/rpadlpar_core.c |1 - 1 files changed, 0 insertions(+), 1 deletions(-) diff --git a/drivers/pci/hotplug/rpadlpar_core.c b/drivers/pci/hotplug/rpadlpar_core.c index b29e20b..bb7af78 100644 --- a/drivers/pci/hotplug/rpadlpar_core.c +++ b/drivers/pci/hotplug/rpadlpar_core.c @@ -388,7 +388,6 @@ int dlpar_remove_pci_slot(char *drc_name, struct device_node *dn) /* Remove the EADS bridge device itself */ BUG_ON(!bus->self); pr_debug("PCI: Now removing bridge device %s\n", pci_name(bus->self)); - eeh_remove_bus_device(bus->self, true); pci_stop_and_remove_bus_device(bus->self); return 0; -- 1.7.5.4 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 02/10] powerpc/eeh: Export functions for hotplug
Make some functions public in order to support hotplug on either specific PCI bus or PCI device in future. Signed-off-by: Gavin Shan --- arch/powerpc/include/asm/eeh.h |9 + arch/powerpc/kernel/eeh.c |6 +++--- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h index 09a8743..d9d35c2 100644 --- a/arch/powerpc/include/asm/eeh.h +++ b/arch/powerpc/include/asm/eeh.h @@ -209,9 +209,12 @@ unsigned long eeh_check_failure(const volatile void __iomem *token, unsigned long val); int eeh_dev_check_failure(struct eeh_dev *edev); void eeh_addr_cache_build(void); +void eeh_add_device_early(struct device_node *); void eeh_add_device_tree_early(struct device_node *); +void eeh_add_device_late(struct pci_dev *); void eeh_add_device_tree_late(struct pci_bus *); void eeh_add_sysfs_files(struct pci_bus *); +void eeh_remove_device(struct pci_dev *, int); void eeh_remove_bus_device(struct pci_dev *, int); /** @@ -252,12 +255,18 @@ static inline unsigned long eeh_check_failure(const volatile void __iomem *token static inline void eeh_addr_cache_build(void) { } +static inline void eeh_add_device_early(struct device_node *dn) { } + static inline void eeh_add_device_tree_early(struct device_node *dn) { } +static inline void eeh_add_device_late(struct pci_dev *dev) { } + 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_device(struct pci_dev *dev, int purge_pe) { } + static inline void eeh_remove_bus_device(struct pci_dev *dev, int purge_pe) { } #define EEH_POSSIBLE_ERROR(val, type) (0) diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c index b5c425e..582ad1e 100644 --- a/arch/powerpc/kernel/eeh.c +++ b/arch/powerpc/kernel/eeh.c @@ -836,7 +836,7 @@ core_initcall_sync(eeh_init); * on the CEC architecture, type of the device, on earlier boot * command-line arguments & etc. */ -static void eeh_add_device_early(struct device_node *dn) +void eeh_add_device_early(struct device_node *dn) { struct pci_controller *phb; @@ -884,7 +884,7 @@ EXPORT_SYMBOL_GPL(eeh_add_device_tree_early); * This routine must be used to complete EEH initialization for PCI * devices that were added after system boot (e.g. hotplug, dlpar). */ -static void eeh_add_device_late(struct pci_dev *dev) +void eeh_add_device_late(struct pci_dev *dev) { struct device_node *dn; struct eeh_dev *edev; @@ -972,7 +972,7 @@ EXPORT_SYMBOL_GPL(eeh_add_sysfs_files); * this device will no longer be detected after this call; thus, * i/o errors affecting this slot may leave this device unusable. */ -static void eeh_remove_device(struct pci_dev *dev, int purge_pe) +void eeh_remove_device(struct pci_dev *dev, int purge_pe) { struct eeh_dev *edev; -- 1.7.5.4 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH v2 0/10] EEH Followup Fixes (II)
The series of patches bases on linux-poerpc-next initially and intends to resolve the following problems: - On pSeries platform, the EEH doesn't work after PHB hotplug with "drmgr". The root cause is that the EEH resources ( EEH devices, EEH caches) aren't released correctly. For the problem, we add one hook (pcibios_stop_dev), which is called on pci_stop_and_remove_device(). In pcibios_stop_dev(), we release the EEH resources. - Another issue is that we need put the domain (PE or PHB) into quite state while doing reset on that domain. However, some deivces in the domain might not have EEH sensitive drivers, or even don't have driver. Those deivces can't be put into quite state and possibly keep issuing PCI-CFG or MMIO request during resetting the domain. That possibly causes the failure of reset and eventually failure of EEH recovery. For the issue, we introduces so-called "partial hotplug". That means, those devices without driver or without EEH sensitive driver are removed before doing reset, and plugged (probed) into the system after reset. - We need traverse EEH devices of one specific PE with safe variant of list tranverse function. The EEH device might be removed while doing iteration. - When doing plug for PCI bus, we need check if we need reassign the resources for subordinate devices (PCI_REASSIGN_ALL_RSRC) and do that accordingly. The patchset is verified on pSeires and PowerNV platforms: pSeries Platform: drmgr -c phb -r -s "PHB 513" drmgr -c phb -a -s "PHB 513" errinjct eeh -f 1 -s net/eth2 PowerNV Platform: cd /sys/devices/pci0005:00/0005:00:00.0/0005:01:00.0/0005:02:08.0/0005:80:00.0/0005:90:01.0 while true; do od -x config > /dev/null; sleep 1; done echo 1 > /sys/kernel/debug/powerpc/PCI0005/err_injct --- v1 -> v2: * Rebase to 3.11.rc1 in order to use pcibios_release_device(). * Use pcibios_release_device() to release EEH cache and detach EEH device from PCI device. * Remove reference to PCI device in EEH cache since we're relying on pcibios_release_device(). * PCI device instance (struct pci_dev) isn't available during BAR restore and avoid use the instance that time. * Fix unbalanced enable for IRQ in eeh_driver.c * Retest the series of patches on Firebird-L/VPL3/VPL4 --- arch/powerpc/include/asm/eeh.h | 28 +- arch/powerpc/include/asm/pci-bridge.h|3 +- arch/powerpc/include/asm/pci.h |2 + arch/powerpc/kernel/eeh.c| 68 +++--- arch/powerpc/kernel/eeh_cache.c | 18 +--- arch/powerpc/kernel/eeh_driver.c | 109 +- arch/powerpc/kernel/eeh_pe.c | 58 +--- arch/powerpc/kernel/eeh_sysfs.c |7 ++ arch/powerpc/kernel/pci-common.c |8 +- arch/powerpc/kernel/pci-hotplug.c| 127 ++ arch/powerpc/kernel/pci_of_scan.c| 43 ++--- arch/powerpc/platforms/powernv/eeh-powernv.c | 11 ++ arch/powerpc/platforms/pseries/eeh_pseries.c | 63 +- drivers/pci/hotplug/rpadlpar_core.c |1 - 14 files changed, 417 insertions(+), 129 deletions(-) Thanks, Gavin ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 01/10] powerpc/eeh: Remove reference to PCI device
We will rely on pcibios_release_device() to remove the EEH cache and unbind EEH device for the specific PCI device. So we shouldn't hold the reference to the PCI device from EEH cache and EEH device. Otherwise, pcibios_release_device() won't be called as we expected. The patch removes the reference to the PCI device in EEH core. Signed-off-by: Gavin Shan --- arch/powerpc/kernel/eeh.c |4 arch/powerpc/kernel/eeh_cache.c | 18 +- 2 files changed, 5 insertions(+), 17 deletions(-) diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c index 39954fe..b5c425e 100644 --- a/arch/powerpc/kernel/eeh.c +++ b/arch/powerpc/kernel/eeh.c @@ -499,8 +499,6 @@ unsigned long eeh_check_failure(const volatile void __iomem *token, unsigned lon } eeh_dev_check_failure(edev); - - pci_dev_put(eeh_dev_to_pci_dev(edev)); return val; } @@ -904,7 +902,6 @@ static void eeh_add_device_late(struct pci_dev *dev) } WARN_ON(edev->pdev); - pci_dev_get(dev); edev->pdev = dev; dev->dev.archdata.edev = edev; @@ -992,7 +989,6 @@ static void eeh_remove_device(struct pci_dev *dev, int purge_pe) } edev->pdev = NULL; dev->dev.archdata.edev = NULL; - pci_dev_put(dev); eeh_rmv_from_parent_pe(edev, purge_pe); eeh_addr_cache_rmv_dev(dev); diff --git a/arch/powerpc/kernel/eeh_cache.c b/arch/powerpc/kernel/eeh_cache.c index f9ac123..e8c9fd5 100644 --- a/arch/powerpc/kernel/eeh_cache.c +++ b/arch/powerpc/kernel/eeh_cache.c @@ -68,16 +68,12 @@ static inline struct eeh_dev *__eeh_addr_cache_get_device(unsigned long addr) struct pci_io_addr_range *piar; piar = rb_entry(n, struct pci_io_addr_range, rb_node); - if (addr < piar->addr_lo) { + if (addr < piar->addr_lo) n = n->rb_left; - } else { - if (addr > piar->addr_hi) { - n = n->rb_right; - } else { - pci_dev_get(piar->pcidev); - return piar->edev; - } - } + else if (addr > piar->addr_hi) + n = n->rb_right; + else + return piar->edev; } return NULL; @@ -156,7 +152,6 @@ eeh_addr_cache_insert(struct pci_dev *dev, unsigned long alo, if (!piar) return NULL; - pci_dev_get(dev); piar->addr_lo = alo; piar->addr_hi = ahi; piar->edev = pci_dev_to_eeh_dev(dev); @@ -250,7 +245,6 @@ restart: if (piar->pcidev == dev) { rb_erase(n, &pci_io_addr_cache_root.rb_root); - pci_dev_put(piar->pcidev); kfree(piar); goto restart; } @@ -302,12 +296,10 @@ void eeh_addr_cache_build(void) if (!edev) continue; - pci_dev_get(dev); /* matching put is in eeh_remove_device() */ dev->dev.archdata.edev = edev; edev->pdev = dev; eeh_addr_cache_insert_dev(dev); - eeh_sysfs_add_device(dev); } -- 1.7.5.4 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH] powerpc: Add second POWER8 PVR entry
POWER8 comes with two different PVRs. This patch enables the additional PVR in the cputable. The existing entry (PVR=0x4b) is renamed to POWER8E and the new entry (PVR=0x4d) is given POWER8. Signed-off-by: Michael Neuling diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h index 5d7d9c2..a6840e4 100644 --- a/arch/powerpc/include/asm/reg.h +++ b/arch/powerpc/include/asm/reg.h @@ -1088,7 +1088,8 @@ #define PVR_970MP 0x0044 #define PVR_970GX 0x0045 #define PVR_POWER7p0x004A -#define PVR_POWER8 0x004B +#define PVR_POWER8E0x004B +#define PVR_POWER8 0x004D #define PVR_BE 0x0070 #define PVR_PA6T 0x0090 diff --git a/arch/powerpc/kernel/cputable.c b/arch/powerpc/kernel/cputable.c index 2a45d0f..22973a7 100644 --- a/arch/powerpc/kernel/cputable.c +++ b/arch/powerpc/kernel/cputable.c @@ -494,9 +494,27 @@ static struct cpu_spec __initdata cpu_specs[] = { .cpu_restore= __restore_cpu_power7, .platform = "power7+", }, - { /* Power8 */ + { /* Power8E */ .pvr_mask = 0x, .pvr_value = 0x004b, + .cpu_name = "POWER8E (raw)", + .cpu_features = CPU_FTRS_POWER8, + .cpu_user_features = COMMON_USER_POWER8, + .cpu_user_features2 = COMMON_USER2_POWER8, + .mmu_features = MMU_FTRS_POWER8, + .icache_bsize = 128, + .dcache_bsize = 128, + .num_pmcs = 6, + .pmc_type = PPC_PMC_IBM, + .oprofile_cpu_type = "ppc64/power8", + .oprofile_type = PPC_OPROFILE_INVALID, + .cpu_setup = __setup_cpu_power8, + .cpu_restore= __restore_cpu_power8, + .platform = "power8", + }, + { /* Power8 */ + .pvr_mask = 0x, + .pvr_value = 0x004d, .cpu_name = "POWER8 (raw)", .cpu_features = CPU_FTRS_POWER8, .cpu_user_features = COMMON_USER_POWER8, diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c index 5eccda9..6079024 100644 --- a/arch/powerpc/kernel/prom_init.c +++ b/arch/powerpc/kernel/prom_init.c @@ -644,7 +644,8 @@ unsigned char ibm_architecture_vec[] = { W(0xfffe), W(0x003a), /* POWER5/POWER5+ */ W(0x), W(0x003e), /* POWER6 */ W(0x), W(0x003f), /* POWER7 */ - W(0x), W(0x004b), /* POWER8 */ + W(0x), W(0x004b), /* POWER8E */ + W(0x), W(0x004d), /* POWER8 */ W(0x), W(0x0f04), /* all 2.07-compliant */ W(0x), W(0x0f03), /* all 2.06-compliant */ W(0x), W(0x0f02), /* all 2.05-compliant */ @@ -706,7 +707,7 @@ unsigned char ibm_architecture_vec[] = { * must match by the macro below. Update the definition if * the structure layout changes. */ -#define IBM_ARCH_VEC_NRCORES_OFFSET117 +#define IBM_ARCH_VEC_NRCORES_OFFSET125 W(NR_CPUS), /* number of cores supported */ 0, 0, ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 4/4 V2] mmc: esdhc: Add broken timeout quirk for p4/p5 board
On 07/17/2013 05:11:31 AM, Haijun Zhang wrote: Sometimes command can't be completed within the time give in eSDHC_SYSCTL[DTOCV]. So just give the max value 0x14 to avoid this issue. Signed-off-by: Haijun Zhang --- changes for v2: - Rebuild patch of eSDHC host need long time to generate command interrupt drivers/mmc/host/sdhci-of-esdhc.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/drivers/mmc/host/sdhci-of-esdhc.c b/drivers/mmc/host/sdhci-of-esdhc.c index 570bca8..30bfb5c 100644 --- a/drivers/mmc/host/sdhci-of-esdhc.c +++ b/drivers/mmc/host/sdhci-of-esdhc.c @@ -325,6 +325,12 @@ static void esdhc_of_platform_init(struct sdhci_host *host) if (vvn > VENDOR_V_22) host->quirks &= ~SDHCI_QUIRK_NO_BUSY_IRQ; + + if ((SVR_SOC_VER(svr) == SVR_B4860) || + (SVR_SOC_VER(svr) == SVR_P5020) || + (SVR_SOC_VER(svr) == SVR_P5040) || + (SVR_SOC_VER(svr) == SVR_P4080)) + host->quirks |= SDHCI_QUIRK_BROKEN_TIMEOUT_VAL; } Please don't line up the continuation lines of the if-condition with the if-body. Please check variant SoCs as well. If the bug exists on p4080, then it exists on p4040. Likewise with p5040/p5021, and p5020/p5010. Is it present on all revisions of these SoCs? How about p3041, which is usually pretty similar to p5020? p2040/p2041? Is there an erratum number for this problem? -Scott ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 3/4 V2] mmc: esdhc: Correct host version of T4240-R1.0
On 07/17/2013 05:11:30 AM, Haijun Zhang wrote: Vender version and sdhc spec version of T4240-R1.0 is incorrect. The right value should be VVN=0x13, SVN = 0x1. The wrong version number will break down the ADMA data transfer. This defect only exist in T4240-R1.0. Will be fixed in T4240-R2.0. Also share vvn and svr for public use. Signed-off-by: Haijun Zhang We're not supporting T4240 rev 1.0 in upstream Linux, as it's not production silicon. -Scott ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v1 01/24] spi: mpc512x: prepare clocks before enabling them
On Wed, Jul 17, 2013 at 04:26:28PM +0200, Gerhard Sittig wrote: > On Wed, Jul 17, 2013 at 13:07 +0100, Mark Brown wrote: > > This is a pretty long e-mail. It'd probably have taken less time to > > fix the issues than to reply to the e-mail... but anyway. > Not quite. Please consider that careful submission is as > expensive as thorough review is, and that a lot of work is done > before v1 gets submitted, which you may not always notice from > looking at a concentrated series that may no longer suggest how > much it took to get there (especially when prepared carefully). This is rather undermined the more time and effort gets spent pushing back against doing trival fixes, of course... besides, the issues here are all really simple to fix and test. It's not a major or risky rewrite of the driver or anything like that - most of this can be tested by just probing the driver. > As mentioned before I did not question the need to fix issues as > they get identified. I just asked whether reworking the serial OK, so send patches then. > The initial COMMON_CLK implementation in part 09/24 registers > clkdev items for compatibility during migration. The string > isn't greppable since it's constructed by the preprocessor in the > MCLK data setup, see the MCLK_SETUP_DATA_PSC() macro. Ugh, right - it didn't show up in searches due to being hidden by the macro. > Now let me see how I can improve the SPI driver with regard to > overall clock handling and beyond mere operation by coincidence > in the absence of errors. But please understand that I don't > want to stall the common clock support for a whole platform just > because some of the drivers it needs to touch to keep them > operational have other issues that weren't addressed yet, while > these issues aren't introduced with the series but preceed it. Again, you're not being asked to implement substantial new functionality here. From my point of view I can't test this at all so I'm looking at code that's obviously not good for the standard clock API and wondering if it even works or how robust it's going to be going forward as the common clock code changes which makes me relucatant to say it'll be OK. The fact that you're switching over to use generic code is itself a reason to make sure that the API usage is sane, dodgy API usage against open coded clock implementations is less risky than against the common code. signature.asc Description: Digital signature ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH] KVM: PPC: Book3S PR: return appropriate error when allocation fails
err was overwritten by a previous function call, and checked to be 0. If the following page allocation fails, 0 is going to be returned instead of -ENOMEM. Signed-off-by: Thadeu Lima de Souza Cascardo --- arch/powerpc/kvm/book3s_pr.c |5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-) diff --git a/arch/powerpc/kvm/book3s_pr.c b/arch/powerpc/kvm/book3s_pr.c index 19498a5..c6e13d9 100644 --- a/arch/powerpc/kvm/book3s_pr.c +++ b/arch/powerpc/kvm/book3s_pr.c @@ -1047,11 +1047,12 @@ struct kvm_vcpu *kvmppc_core_vcpu_create(struct kvm *kvm, unsigned int id) if (err) goto free_shadow_vcpu; + err = -ENOMEM; p = __get_free_page(GFP_KERNEL|__GFP_ZERO); - /* the real shared page fills the last 4k of our page */ - vcpu->arch.shared = (void*)(p + PAGE_SIZE - 4096); if (!p) goto uninit_vcpu; + /* the real shared page fills the last 4k of our page */ + vcpu->arch.shared = (void *)(p + PAGE_SIZE - 4096); #ifdef CONFIG_PPC_BOOK3S_64 /* default to book3s_64 (970fx) */ -- 1.7.1 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v1 01/24] spi: mpc512x: prepare clocks before enabling them
On Wed, Jul 17, 2013 at 13:07 +0100, Mark Brown wrote: > > On Wed, Jul 17, 2013 at 01:22:29PM +0200, Gerhard Sittig wrote: > > On Mon, Jul 15, 2013 at 21:17 +0100, Mark Brown wrote: > > > On Mon, Jul 15, 2013 at 08:47:30PM +0200, Gerhard Sittig wrote: > > > > > sprintf(name, "psc%d_mclk", master->bus_num); > > > > spiclk = clk_get(&master->dev, name); > > > > - clk_enable(spiclk); > > > > + clk_prepare_enable(spiclk); > > > > mps->mclk = clk_get_rate(spiclk); > > > > clk_put(spiclk); > > > > This code is *clearly* buggy and should be fixed rather than papered > > > over. Not only is there no matching put the driver is also dropping the > > > reference to the clock rather than holding on to it while it's in use. > > > "This code" refers to the driver's original condition, right? I > > agree that the driver has been suffering from incomplete clock > > handling since it was born three years ago. And that this issue > > shall get addressed. The question is just whether it needs to be > > part of this series which has a different focus. > > This is a pretty long e-mail. It'd probably have taken less time to > fix the issues than to reply to the e-mail... but anyway. [ this is meta stuff, technical details are after the next quotation ] Not quite. Please consider that careful submission is as expensive as thorough review is, and that a lot of work is done before v1 gets submitted, which you may not always notice from looking at a concentrated series that may no longer suggest how much it took to get there (especially when prepared carefully). As mentioned before I did not question the need to fix issues as they get identified. I just asked whether reworking the serial driver is in the scope of this series which provides a different clock driver implementation for the platform. To me these two things are orthogonal, while I did promise to see how I can address your concerns. Let's provide the technical information you need to judge the quality of the submission and see why it shall be acceptable: > A big part of the issue with the state of the driver is that there's > obvious clock API abuse going on that isn't corrected here - the main > one is that the sprintf() for the clock name is a fairly clear warning > sign, the driver should be using a fixed value there. This raises a > warning flag for me about the quality of the common clock API > implementation that's being sent and/or the potential for bugs to be > noticed with the common clock framework. I'd not expect this code to > actually work, and looking at the rest of the series I don't see how it > does since I can't see what forces the name. Why the code does work: OK, here is why the driver keeps its state throughout the set and is operational as good or as bad as it was before: The sprintf() in the SPI driver used to construct a name which includes the PSC index. This applies to the UART driver as well, as they both use the PSC controller hardware to implement their serial communication. The corresponding clock name was found in the previous PPC_CLOCK implementation since the clock driver provided those names which include the PSC index (fixed strings in a table). The initial COMMON_CLK implementation in part 09/24 registers clkdev items for compatibility during migration. The string isn't greppable since it's constructed by the preprocessor in the MCLK data setup, see the MCLK_SETUP_DATA_PSC() macro. Part 13/24 adjusts the SPI driver to no longer construct a name, but instead use a fixed string after OF based clock lookup became available. This shall meet your expectation and simplifies the client side. Part 15/24 does the same for the UART driver. Part 16/24 removes the clkdev registration in the clock driver after both the UART and SPI clients switched to OF clock lookups. At this stage things are the way you would expect: The client uses a fixed string in the lookup, while lookup occurs via device tree, and instances are supported without the clients' constructing something based on a component index. Remaining issues are not with the common clock support of the platform, but in the serial driver and are identical to the previous (actually: the initial) status. While we agree on the existance of the remaining issue and the desire to address it. Just not on which context that fix shall be done in. The series' status and perspective: The UART and SPI drivers did work before, while it's true that clock handling wasn't complete (in non-fatal ways). This has been the case in the past, and has been identified just now. The serial drivers are kept operational during migration, and still are operational after the series. But now they use common clock and OF lookups, which is an improvement for the platform in my eyes. It's true that the status of the serial driver isn't improved with the series -- but it isn't degraded either, while (additional) breakage actively gets prevented.
Re: [PATCH v1 01/24] spi: mpc512x: prepare clocks before enabling them
On Wed, Jul 17, 2013 at 01:22:29PM +0200, Gerhard Sittig wrote: > On Mon, Jul 15, 2013 at 21:17 +0100, Mark Brown wrote: > > On Mon, Jul 15, 2013 at 08:47:30PM +0200, Gerhard Sittig wrote: > > > sprintf(name, "psc%d_mclk", master->bus_num); > > > spiclk = clk_get(&master->dev, name); > > > - clk_enable(spiclk); > > > + clk_prepare_enable(spiclk); > > > mps->mclk = clk_get_rate(spiclk); > > > clk_put(spiclk); > > This code is *clearly* buggy and should be fixed rather than papered > > over. Not only is there no matching put the driver is also dropping the > > reference to the clock rather than holding on to it while it's in use. > "This code" refers to the driver's original condition, right? I > agree that the driver has been suffering from incomplete clock > handling since it was born three years ago. And that this issue > shall get addressed. The question is just whether it needs to be > part of this series which has a different focus. This is a pretty long e-mail. It'd probably have taken less time to fix the issues than to reply to the e-mail... but anyway. A big part of the issue with the state of the driver is that there's obvious clock API abuse going on that isn't corrected here - the main one is that the sprintf() for the clock name is a fairly clear warning sign, the driver should be using a fixed value there. This raises a warning flag for me about the quality of the common clock API implementation that's being sent and/or the potential for bugs to be noticed with the common clock framework. I'd not expect this code to actually work, and looking at the rest of the series I don't see how it does since I can't see what forces the name. signature.asc Description: Digital signature ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v1 05/24] clk: wrap I/O access for improved portability
On Mon, Jul 15, 2013 at 21:38 +0200, Sascha Hauer wrote: > > On Mon, Jul 15, 2013 at 08:47:34PM +0200, Gerhard Sittig wrote: > > diff --git a/drivers/clk/clk-divider.c b/drivers/clk/clk-divider.c > > index 6d55eb2..2c07061 100644 > > --- a/drivers/clk/clk-divider.c > > +++ b/drivers/clk/clk-divider.c > > @@ -104,7 +104,7 @@ static unsigned long clk_divider_recalc_rate(struct > > clk_hw *hw, > > struct clk_divider *divider = to_clk_divider(hw); > > unsigned int div, val; > > > > - val = readl(divider->reg) >> divider->shift; > > + val = clk_readl(divider->reg) >> divider->shift; > > val &= div_mask(divider); > > Would it be an option to use regmap for the generic dividers/muxes > instead? This should be suitable for ppc and also for people who want to > use the generic clocks on i2c devices. Does regmap assume that those registers form a (dense) array of equal sized items? Does it introduce unconditional indirection and requirement for explicit setup beyond mere mapping? What's the overhead compared to the readl/inbe32 approach that's currently resolved at compile time? Neither of the above needs to be a blocker, just needs to be acceptable after consideration. So far nobody appears to have felt pain with the LE32 register assumption. BTW does the common clock support that I introduce for MPC512x only cover those parts from crystal over internal busses to internal peripherals (register files). It does not include bitrate generation within the peripherals -- this appears to remain the domain of individual drivers, which keep manipulating register fields to derive wire bitrates from internal references. Sharing drivers between platforms with and without common clock support forbids the switch anyway, or both CCF abstraction as well as local register manipulation need to get implemented in parallel, as I did for the mscan(4) driver. Some of these bitrate related registers aren't 32bit entities and thus could not get managed by the common clock primitives in their current form. Some of the IP blocks may even spread integer values across several non-adjacent bit fields for legacy reasons, but I guess that these aren't in the scope of the shared primitives either (and they may not be popular either). While we are at improvements for the common clock primitives: What I missed was support for fractional dividers, i.e. dividers with a register bitfield backed divider part but a fixed factor multiplier part. Currently there's only dividers (bitfield factor or bitfield with table lookup for the factor) and fixed-factor (multiplier and divider, but both of them fixed and not adjustable by register manipulation). This was addressed by "intermediate" clock items ("ungated", "x4") virtually yours Gerhard Sittig -- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr. 5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: off...@denx.de ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v1 15/24] serial: mpc512x: OF clock lookup, use the 'mclk' name
On Mon, Jul 15, 2013 at 23:54 +0200, Sascha Hauer wrote: > > On Mon, Jul 15, 2013 at 11:46:01PM +0200, Gerhard Sittig wrote: > > with device tree based clock lookup, the MCLK name no longer > > depends on the PSC index > > > > Signed-off-by: Gerhard Sittig > > --- > > drivers/tty/serial/mpc52xx_uart.c |8 ++-- > > 1 file changed, 2 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/tty/serial/mpc52xx_uart.c > > b/drivers/tty/serial/mpc52xx_uart.c > > index 53c1093..221fb89 100644 > > --- a/drivers/tty/serial/mpc52xx_uart.c > > +++ b/drivers/tty/serial/mpc52xx_uart.c > > @@ -619,21 +619,17 @@ static irqreturn_t mpc512x_psc_handle_irq(struct > > uart_port *port) > > static int mpc512x_psc_clock(struct uart_port *port, int enable) > > { > > struct clk *psc_clk; > > - int psc_num; > > - char clk_name[10]; > > > > if (uart_console(port)) > > return 0; > > > > - psc_num = (port->mapbase & 0xf00) >> 8; > > - snprintf(clk_name, sizeof(clk_name), "psc%d_mclk", psc_num); > > - psc_clk = clk_get(port->dev, clk_name); > > + psc_clk = clk_get(port->dev, "mclk"); > > Same comment applies here as Mark made to the spi driver. So I'd like to respond in the same way as I did for the SPI driver. :) The scope of this series is the introduction of support for the common clock framework. Addressing other (legacy non-fatal and previously accepted) issues as they get identified in bypassing would be the scope of a separate patch or series. I'm not questioning the need to fix other additionally identified issues. I'm just asking whether they shall be in the scope of this very series. virtually yours Gerhard Sittig -- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr. 5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: off...@denx.de ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v1 01/24] spi: mpc512x: prepare clocks before enabling them
On Mon, Jul 15, 2013 at 21:17 +0100, Mark Brown wrote: > > On Mon, Jul 15, 2013 at 08:47:30PM +0200, Gerhard Sittig wrote: > > clocks need to get prepared before they can get enabled, > > fix the MPC512x PSC SPI master's initialization > > > Signed-off-by: Gerhard Sittig > > --- > > drivers/spi/spi-mpc512x-psc.c |2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/spi/spi-mpc512x-psc.c b/drivers/spi/spi-mpc512x-psc.c > > index 29fce6a..76b20ea 100644 > > --- a/drivers/spi/spi-mpc512x-psc.c > > +++ b/drivers/spi/spi-mpc512x-psc.c > > @@ -395,7 +395,7 @@ static int mpc512x_psc_spi_port_config(struct > > spi_master *master, > > > > sprintf(name, "psc%d_mclk", master->bus_num); > > spiclk = clk_get(&master->dev, name); > > - clk_enable(spiclk); > > + clk_prepare_enable(spiclk); > > mps->mclk = clk_get_rate(spiclk); > > clk_put(spiclk); > > This code is *clearly* buggy and should be fixed rather than papered > over. Not only is there no matching put the driver is also dropping the > reference to the clock rather than holding on to it while it's in use. "This code" refers to the driver's original condition, right? I agree that the driver has been suffering from incomplete clock handling since it was born three years ago. And that this issue shall get addressed. The question is just whether it needs to be part of this series which has a different focus. What the above patch addresses is prevention of an immediate and fatal breakage, when common clock gets used but clocks aren't prepared before getting enabled. So I consider it appropriate as a step in preparation before introducing support for common clock on the platform. (It's funny that I had a comment in this spirit in the commit message, but trimmed it to not be overly verbose.) What the patch does not address is to fix any other deficiencies in the driver which might have been lurking there for ages. This would be the scope of a different patch or series, as addressing it here as well would mix orthogonal issues within one series, and would complicate review and test (and would delay or even potentially kill the introduction of desirable support for common infrastructure just because other legacy non-fatal issues aren't addressed as well in bypassing). I will look into what I can do to address your concerns about proper general clock handling in this specific driver. But I'm unable to do this for all other drivers as well which I happen to pass by as I work on platform code (partially due to lack of knowledge). In any case I won't be able to test all of these changes in all subsystems (mostly due to lack of hardware and test setups). So I suggest to leave these general cleanup activities for a separate series. The current series certainly improves general platform code, transparently brings new drivers into successful operation, and keeps the quality of existing code (doesn't break anything, does even actively prevent breakage). So it shall be acceptable despite its not being perfect (like addressing each and every other aspect which may arise elsewhere). Where would I stop then? Sorry for the lengthy reply, but I guess it's about just one general aspect which equally applies to other parts of the series, not just the specific SPI driver which the feedback was provided for. And I do appreciate your looking at the submission. virtually yours Gerhard Sittig -- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr. 5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: off...@denx.de ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 4/4 V2] mmc: esdhc: Add broken timeout quirk for p4/p5 board
Sometimes command can't be completed within the time give in eSDHC_SYSCTL[DTOCV]. So just give the max value 0x14 to avoid this issue. Signed-off-by: Haijun Zhang --- changes for v2: - Rebuild patch of eSDHC host need long time to generate command interrupt drivers/mmc/host/sdhci-of-esdhc.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/drivers/mmc/host/sdhci-of-esdhc.c b/drivers/mmc/host/sdhci-of-esdhc.c index 570bca8..30bfb5c 100644 --- a/drivers/mmc/host/sdhci-of-esdhc.c +++ b/drivers/mmc/host/sdhci-of-esdhc.c @@ -325,6 +325,12 @@ static void esdhc_of_platform_init(struct sdhci_host *host) if (vvn > VENDOR_V_22) host->quirks &= ~SDHCI_QUIRK_NO_BUSY_IRQ; + + if ((SVR_SOC_VER(svr) == SVR_B4860) || + (SVR_SOC_VER(svr) == SVR_P5020) || + (SVR_SOC_VER(svr) == SVR_P5040) || + (SVR_SOC_VER(svr) == SVR_P4080)) + host->quirks |= SDHCI_QUIRK_BROKEN_TIMEOUT_VAL; } static int esdhc_pltfm_bus_width(struct sdhci_host *host, int width) -- 1.8.0 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 3/4 V2] mmc: esdhc: Correct host version of T4240-R1.0
Vender version and sdhc spec version of T4240-R1.0 is incorrect. The right value should be VVN=0x13, SVN = 0x1. The wrong version number will break down the ADMA data transfer. This defect only exist in T4240-R1.0. Will be fixed in T4240-R2.0. Also share vvn and svr for public use. Signed-off-by: Haijun Zhang --- changes for V2: - Remove broken ADMA quirk. - Rebuild patch of Add quirks to support T4240 board drivers/mmc/host/sdhci-of-esdhc.c | 29 + 1 file changed, 13 insertions(+), 16 deletions(-) diff --git a/drivers/mmc/host/sdhci-of-esdhc.c b/drivers/mmc/host/sdhci-of-esdhc.c index adfaadd..570bca8 100644 --- a/drivers/mmc/host/sdhci-of-esdhc.c +++ b/drivers/mmc/host/sdhci-of-esdhc.c @@ -26,7 +26,7 @@ #define VENDOR_V_220x12 #define VENDOR_V_230x13 -static u32 svr; +static u32 svr, vvn; static u32 esdhc_readl(struct sdhci_host *host, int reg) { @@ -43,11 +43,9 @@ static u32 esdhc_readl(struct sdhci_host *host, int reg) * For FSL eSDHC, must aligned 4-byte, so use 0xFC to read the * the verdor version number, oxFE is SDHCI_HOST_VERSION. */ - if ((reg == SDHCI_CAPABILITIES) && (ret & SDHCI_CAN_DO_ADMA1)) { - u32 tmp = in_be32(host->ioaddr + SDHCI_SLOT_INT_STATUS); - tmp = (tmp & SDHCI_VENDOR_VER_MASK) >> SDHCI_VENDOR_VER_SHIFT; - if (tmp > VENDOR_V_22) - ret |= SDHCI_CAN_DO_ADMA2; + if ((reg == SDHCI_CAPABILITIES) && (ret & SDHCI_CAN_DO_ADMA1) && + (vvn > VENDOR_V_22)) { + ret |= SDHCI_CAN_DO_ADMA2; } return ret; @@ -63,6 +61,12 @@ static u16 esdhc_readw(struct sdhci_host *host, int reg) ret = in_be32(host->ioaddr + base) & 0x; else ret = (in_be32(host->ioaddr + base) >> shift) & 0x; + + /* T4240-R1.0 had a incorrect vendor version and spec version */ + if ((reg == SDHCI_HOST_VERSION) && + ((SVR_SOC_VER(svr) == SVR_T4240) && (SVR_REV(svr) == 0x10))) + ret = (VENDOR_V_23 << SDHCI_VENDOR_VER_SHIFT) | SDHCI_SPEC_200; + return ret; } @@ -175,17 +179,12 @@ static void esdhc_reset(struct sdhci_host *host, u8 mask) */ static void esdhci_of_adma_workaround(struct sdhci_host *host, u32 intmask) { - u32 tmp; bool applicable; dma_addr_t dmastart; dma_addr_t dmanow; - tmp = esdhc_readl(host, SDHCI_SLOT_INT_STATUS); - tmp = (tmp & SDHCI_VENDOR_VER_MASK) >> SDHCI_VENDOR_VER_SHIFT; - applicable = (intmask & SDHCI_INT_DATA_END) && - (intmask & SDHCI_INT_BLK_GAP) && - (tmp == VENDOR_V_23); + (intmask & SDHCI_INT_BLK_GAP) && (vvn == VENDOR_V_23); if (applicable) { esdhc_reset(host, SDHCI_RESET_DATA); @@ -318,10 +317,9 @@ static void esdhc_of_resume(struct sdhci_host *host) static void esdhc_of_platform_init(struct sdhci_host *host) { - u32 vvn; + svr = mfspr(SPRN_SVR); + vvn = esdhc_readw(host, SDHCI_HOST_VERSION); - vvn = in_be32(host->ioaddr + SDHCI_SLOT_INT_STATUS); - vvn = (vvn & SDHCI_VENDOR_VER_MASK) >> SDHCI_VENDOR_VER_SHIFT; if (vvn == VENDOR_V_22) host->quirks2 |= SDHCI_QUIRK2_HOST_NO_CMD23; @@ -390,7 +388,6 @@ static int sdhci_esdhc_probe(struct platform_device *pdev) struct device_node *np; int ret; - svr = mfspr(SPRN_SVR); host = sdhci_pltfm_init(pdev, &sdhci_esdhc_pdata, 0); if (IS_ERR(host)) return PTR_ERR(host); -- 1.8.0 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 2/4 V2] mmc: esdhc: workaround for dma err in the last system transaction
A-004388: eSDHC DMA might not stop if error occurs on system transaction eSDHC DMA(SDMA/ADMA) might not stop if an error occurs in the last system transaction. It may continue initiating additional transactions until software reset for data/all is issued during error recovery. There is not any data corruption to the SD data. The IRQSTAT[DMAE] is set when the erratum occurs. The only conditions under which issues occur are the following: 1. SDMA - For SD Write , the error occurs in the last system transaction. No issue for SD read 2. ADMA a. Block count is enabled: For SD write, the error occurs in the last system transaction. There is no issue for SD read when block count is enabled. b. Block count is disabled: Block count is designated by the ADMA descriptor table, and the error occurs in the last system transaction when ADMA is executing last descriptor line of table. eSDHC may initiate additional system transactions. There is no data integrity issue for case 1 and 2a described below. For case 2b, system data might be corrupted. Workaround: Set eSDHC_SYSCTL[RSTD] when IRQSTAT[DMAE] is set. For cases 2a and 2b above, add an extra descriptor line with zero data next to the last descriptor line. Signed-off-by: Haijun Zhang --- changes for V2: - Update the svr version list drivers/mmc/host/sdhci-of-esdhc.c | 112 ++ 1 file changed, 102 insertions(+), 10 deletions(-) diff --git a/drivers/mmc/host/sdhci-of-esdhc.c b/drivers/mmc/host/sdhci-of-esdhc.c index 15039e2..adfaadd 100644 --- a/drivers/mmc/host/sdhci-of-esdhc.c +++ b/drivers/mmc/host/sdhci-of-esdhc.c @@ -21,9 +21,13 @@ #include #include "sdhci-pltfm.h" #include "sdhci-esdhc.h" +#include #define VENDOR_V_220x12 #define VENDOR_V_230x13 + +static u32 svr; + static u32 esdhc_readl(struct sdhci_host *host, int reg) { u32 ret; @@ -142,6 +146,26 @@ static void esdhc_writeb(struct sdhci_host *host, u8 val, int reg) sdhci_be32bs_writeb(host, val, reg); } +static void esdhc_reset(struct sdhci_host *host, u8 mask) +{ + u32 ier; + u32 uninitialized_var(isav); + + if (host->quirks & SDHCI_QUIRK_RESTORE_IRQS_AFTER_RESET) + isav = esdhc_readl(host, SDHCI_INT_ENABLE); + + esdhc_writeb(host, mask, SDHCI_SOFTWARE_RESET); + mdelay(100); + + if (host->quirks & SDHCI_QUIRK_RESTORE_IRQS_AFTER_RESET) { + ier = esdhc_readl(host, SDHCI_INT_ENABLE); + ier &= ~SDHCI_INT_ALL_MASK; + ier |= isav; + esdhc_writel(host, ier, SDHCI_INT_ENABLE); + esdhc_writel(host, ier, SDHCI_SIGNAL_ENABLE); + } +} + /* * For Abort or Suspend after Stop at Block Gap, ignore the ADMA * error(IRQSTAT[ADMAE]) if both Transfer Complete(IRQSTAT[TC]) @@ -156,25 +180,92 @@ static void esdhci_of_adma_workaround(struct sdhci_host *host, u32 intmask) dma_addr_t dmastart; dma_addr_t dmanow; - tmp = in_be32(host->ioaddr + SDHCI_SLOT_INT_STATUS); + tmp = esdhc_readl(host, SDHCI_SLOT_INT_STATUS); tmp = (tmp & SDHCI_VENDOR_VER_MASK) >> SDHCI_VENDOR_VER_SHIFT; applicable = (intmask & SDHCI_INT_DATA_END) && (intmask & SDHCI_INT_BLK_GAP) && (tmp == VENDOR_V_23); - if (!applicable) + if (applicable) { + + esdhc_reset(host, SDHCI_RESET_DATA); + host->data->error = 0; + dmastart = sg_dma_address(host->data->sg); + dmanow = dmastart + host->data->bytes_xfered; + + /* Force update to the next DMA block boundary. */ + dmanow = (dmanow & ~(SDHCI_DEFAULT_BOUNDARY_SIZE - 1)) + + SDHCI_DEFAULT_BOUNDARY_SIZE; + host->data->bytes_xfered = dmanow - dmastart; + esdhc_writel(host, dmanow, SDHCI_DMA_ADDRESS); + return; + } - host->data->error = 0; - dmastart = sg_dma_address(host->data->sg); - dmanow = dmastart + host->data->bytes_xfered; /* -* Force update to the next DMA block boundary. +* Check for A-004388: eSDHC DMA might not stop if error +* occurs on system transaction +* Impact list: +* T4240-R1.0 B4860-R1.0 P1010-R1.0 +* P3041-R1.0-R2.0-R1.1 P2041-R1.0-R1.1-R2.0 +* P5040-R2.0 */ - dmanow = (dmanow & ~(SDHCI_DEFAULT_BOUNDARY_SIZE - 1)) + - SDHCI_DEFAULT_BOUNDARY_SIZE; - host->data->bytes_xfered = dmanow - dmastart; - sdhci_writel(host, dmanow, SDHCI_DMA_ADDRESS); + if (!(((SVR_SOC_VER(svr) == SVR_T4240) && (SVR_REV(svr) == 0x10)) || + ((SVR_SOC_VER(svr) == SVR_B4860) && (SVR_REV(svr) == 0x10)) || + ((SVR_SOC_VER(svr) == SVR_P1010) && (SVR_REV(svr) == 0x10)) || + ((SVR_SOC_VER(svr) == SVR_P3041) && (SVR_REV(svr) <= 0x20)) || + ((SVR_SOC_VER(svr) == SVR_P2041) && (SVR_REV(svr) <= 0x2
Re: [PATCH RFC v2 2/5] dma: mpc512x: add support for peripheral transfers
On Tue, Jul 16, 2013 at 12:37 +0200, Lars-Peter Clausen wrote: > > On 07/14/2013 02:01 PM, Gerhard Sittig wrote: > > From: Alexander Popov > > > > introduce support for slave s/g transfer preparation and the associated > > device control callback in the MPC512x DMA controller driver, which adds > > support for data transfers between memory and peripheral I/O to the > > previously supported mem-to-mem transfers > > > > refuse to prepare chunked transfers (transfers with more than one part) > > as long as proper support for scatter/gather is lacking > > > > keep MPC8308 operational by always starting transfers from software, > > this SoC appears to not have request lines for flow control when > > peripherals are involved in transfers > > I had a look at the current driver and it seems that any channel can be used > for memcpy operation not only the MDDRC channel. Since the dmaengine API > will just pick one of the currently free channels when performing a memcpy > operation I think this patch breaks memcpy operations. You probably need to > register two dma controllers, one for memcpy operations one for slave > operations, that way you can ensure that only the MDDRC channel is used for > memcpy operations. OK, so the need for explicit start in software or external request by the peripheral remains, but the condition for the choice is different. It might become a flag attached to the DMA channel, that gets setup in the prep routines (memory and slave access each use their own prep calls) and gets evaluated in execute. Something like "will access memory", or "will access peripheral" (the latter may be preferrable since slave support is the new feature). This assumes that the non-MDDRC channels can get used for memory transfers as well, which your description of the the former use suggests, when slave support was absent. Making the MDDRC channel preferrable for memcpy operations is orthogonal to that. > > > > [ introduction of slave s/g preparation and device control ] > > Signed-off-by: Alexander Popov > > [ execute() start condition, mpc8308 compat, terminate all, s/g length > > check, reworded commit msg ] > > Signed-off-by: Gerhard Sittig > > --- > [...] > > + > > +static int mpc_dma_device_control(struct dma_chan *chan, enum dma_ctrl_cmd > > cmd, > > + unsigned long arg) > > +{ > > + struct mpc_dma_chan *mchan; > > + struct mpc_dma *mdma; > > + struct dma_slave_config *cfg; > > + > > + mchan = dma_chan_to_mpc_dma_chan(chan); > > + switch (cmd) { > > + case DMA_TERMINATE_ALL: > > + /* disable channel requests */ > > + mdma = dma_chan_to_mpc_dma(chan); > > + out_8(&mdma->regs->dmacerq, chan->chan_id); > > + list_splice_tail_init(&mchan->prepared, &mchan->free); > > + list_splice_tail_init(&mchan->queued, &mchan->free); > > + list_splice_tail_init(&mchan->active, &mchan->free); > > This probably need locking. Ah, yes. It needs to grab the same lock as all the other list manipulations in the driver's source. virtually yours Gerhard Sittig -- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr. 5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: off...@denx.de ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [V2 2/2] powerpc/512x: add LocalPlus Bus FIFO device driver
On Fri, Jul 12, 2013 at 14:42 +0400, Alexander Popov wrote: > > 2013/7/10 Gerhard Sittig : > > On Wed, Jul 10, 2013 at 14:21 +0400, Alexander Popov wrote: > >> > >> + > >> +struct mpc512x_lpbfifo_request { > >> + unsigned int cs; > >> + phys_addr_t bus_phys; /* physical address of some device on lpb */ > >> + void *ram_virt; /* virtual address of some region in ram */ > >> + > >> + /* Details of transfer */ > >> + u32 size; > >> + enum lpb_dev_portsize portsize; > >> + enum mpc512x_lpbfifo_req_dir dir; > >> + > >> + /* Call when the transfer is finished */ > >> + void (*callback)(struct mpc512x_lpbfifo_request *); > >> +}; > >> + > >> +extern int mpc512x_lpbfifo_submit(struct mpc512x_lpbfifo_request *req); > >> + > >> #endif /* __ASM_POWERPC_MPC5121_H__ */ > > > Needs the mpc512x_lpbfifo_request structure be part of the > > official API? Could it be desirable to hide it behind a > > "fill-in" routine? Which BTW could auto-determine CS numbers and > > port width associated with a chip select from the XLB and LPB > > register set? > > 1. As I wrote at the beginning of the file, the driver design > is based on mpc52xx_lpbfifo driver written by Grant Likely. > > 2. CS and port width are attributes of some device on LPBus > which participates DMA data transfer. It is the driver of this device > who knows them. I don't think so (I disagree with the "driver knows physical bus attributes" part). Bus width, endianess, timing in access, etc are all properties of a chip select. And the chip select is associated with an address range. See the CS part of the LPB controller, and the LAW part of the XLB. Given a physical address, everything else can get determined from inspecting the SoC's registers. Just think what the SoC does: "Someone", probably the CPU or a bus master like DMA, accesses an address, which happens to be within an address window, which happens to be connected to some bus and maybe map to a chip select, which happens to use the chip select's configuration for the activity on the associated bus -- upon memory access nobody needs to explicitly know whether SRAM or DRAM or IMMR or LPB aka EMB is involved, it's all hidden within the XLB logic. I feel that the SCLPC job description is an exception, and the need to specify a CS index is the unusual case. Drivers actually _need_not_ know the CS number or bus width of what's attached to the EMB. Those parameters usually get setup within boot loaders and the kernel need not care. Linux drivers merely map an address range and "just access memory that happens to have some arbitrary address". Drivers need not care whether the bus is internal or external nor whether the bus has several chip selects nor how these might be configured. The recent addition to re-configure a chip select already was a special case of differing configuration during runtime (when the firmware gets sent, and when the firmware is used and communicated to), and is by no means the normal case. > It fills mpc512x_lpbfifo_request as a client side of that API. > > > Who's using the submit routine? I might have missed the "client > > side" of that API in the series. > > Please look at https://patchwork.kernel.org/patch/2511951/ This answers how the current implementation is, not necessarily how the API should or might be. But I wasn't requesting immediate change, just was pointing at potential for improvement. > >> +#define LPBFIFO_REG_PACKET_SIZE (0x00) > >> +#define LPBFIFO_REG_START_ADDRESS(0x04) > >> +#define LPBFIFO_REG_CONTROL (0x08) > >> +#define LPBFIFO_REG_ENABLE (0x0C) > >> +#define LPBFIFO_REG_STATUS (0x14) > >> +#define LPBFIFO_REG_BYTES_DONE (0x18) > >> +#define LPBFIFO_REG_EMB_SHARE_COUNTER(0x1C) > >> +#define LPBFIFO_REG_EMB_PAUSE_CONTROL(0x20) > >> +#define LPBFIFO_REG_FIFO_DATA(0x40) > >> +#define LPBFIFO_REG_FIFO_STATUS (0x44) > >> +#define LPBFIFO_REG_FIFO_CONTROL (0x48) > >> +#define LPBFIFO_REG_FIFO_ALARM (0x4C) > > > Should this not be a struct? Since using member field names > > allows for compile time checks of data types, which is highly > > desirable with registers of arbitrarily differing size. > > I've read > https://lists.ozlabs.org/pipermail/linuxppc-dev/2010-January/079476.html > Which way should I use? The way Grant puts it, there are valid reasons for both approaches and things may turn out to be a matter of preference. As is written in the post you refer to, neither of them is in itself right or wrong and leads to immediate rejection, it's just that one of them appears more appropriate in a specific situation. Given that we are dealing with SoC internal IP peripherals, their physical attachment is known and stable, the register layout and data width is a given. The phrase out_be32(®s->field, val) is so much more readable than fiddling with a base and offsets. Data
Re: [PATCH V2 1/2] DMA: Freescale: Add new 8-channel DMA engine device tree nodes
On 07/15/2013 09:31 PM, Kumar Gala wrote: On Jul 5, 2013, at 1:27 AM, wrote: From: Hongbo Zhang Freescale QorIQ T4 and B4 introduce new 8-channel DMA engines, this patch add the device tree nodes for them. Signed-off-by: Hongbo Zhang --- arch/powerpc/boot/dts/fsl/elo3-dma-0.dtsi | 90 +++ arch/powerpc/boot/dts/fsl/elo3-dma-1.dtsi | 90 +++ arch/powerpc/boot/dts/fsl/t4240si-post.dtsi |4 +- 3 files changed, 182 insertions(+), 2 deletions(-) create mode 100644 arch/powerpc/boot/dts/fsl/elo3-dma-0.dtsi create mode 100644 arch/powerpc/boot/dts/fsl/elo3-dma-1.dtsi Why didn't you update b4si-post.dtsi as well? OK, will update it too, thanks. - k ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev