Re: [alsa-devel] [PATCH v4 1/2] ASoC: fsl: Add S/PDIF CPU DAI driver
On Wed, Aug 14, 2013 at 02:39:33PM +0800, Shawn Guo wrote: > We only need to maintain those versions that require different > programming model in the list. For example, if S/PDIF on Vybrid > is completely compatible with imx6q one and uses the exactly same > programming model, we do not need to maintain a compatible string > for Vybrid S/PDIF at all. Instead, we only need to have something > like below in Vybrid dts file, and S/PDIF driver will just work for it. > > compatible = "fsl,vf600-spdif", "fsl,imx6q-spdif"; > > Shawn Clear. Thank you for the explain. Then I think I can merely remain "fsl,imx6q-spdif" here, because all other cases should be completely compatible with this one. They are only different in the clock source names list, which's already being specified in dts file. Please correct me if you think this still isn't proper. Best regards, Nicolin Chen ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v4 resent 1/2] ASoC: fsl: Add S/PDIF CPU DAI driver
On Mon, Aug 12, 2013 at 08:05:27PM +0800, Nicolin Chen wrote:
> This patch add S/PDIF controller driver for Freescale SoC.
>
> Signed-off-by: Nicolin Chen
> +
> +Required properties:
> +
> + - compatible : Compatible list, contains "fsl,-spdif". Using general
> + "fsl,fsl-spdif" will get the default SoC type -- imx6q-spdif.
> +
> + - reg : Offset and length of the register set for the device.
> +
> + - interrupts : Contains spdif interrupt.
> +
> + - dmas : Generic dma devicetree binding as described in
> + Documentation/devicetree/bindings/dma/dma.txt.
> +
> + - dma-names : Two dmas have to be defined, "tx" and "rx".
> +
> + - clocks : Contains an entry for each entry in clock-names.
> +
> + - clock-names : Includes the following entries:
> + nametypecomments
> + "core" RequiredThe core clock of spdif controller
> + "rx"OptionalRx clock source for spdif record.
> + If absent, will use core clock.
> + "tx"OptionalTx clock source for spdif playback.
> + If absent, will use core clock.
> + "tx-32000" OptionalTx clock source for 32000Hz sample rate
> + playback. If absent, will use tx clock.
> + "tx-44100" OptionalTx clock source for 44100Hz sample rate
> + playback. If absent, will use tx clock.
> + "tx-48000" OptionalTx clock source for 48000Hz sample rate
> + playback. If absent, will use tx clock.
> +
> + - tx-clksrc-names : The names for all available clock sources for tx, which
> + is also being listed in SoC reference manual, ClkSrc_Sel bit of SPDIF_SRPC.
> + And the name list would be different between different SoC. Use 'null' for
> + those unlisted names, and the max number of tx-clksrc-names should be 8.
> +
> + - rx-clksrc-names : The names for all available clock sources for rx, which
> + is also being listed in SoC reference manual, TxClk_Source bit of
> SPDIF_STC.
> + And the name list would be different between different SoC. Use 'null' for
> + those unlisted names, and the max number of rx-clksrc-names should be 16.
> +
> +Optional properties:
> +
> + - rx-clksrc-lock: This is a boolean property. If present, ClkSrc_Sel bit
> + of SPDIF_SRPC would be set a clock source that cares DPLL locked condition.
> +
> +Example1:
> +
> +spdif: spdif@02004000 {
> + compatible = "fsl,imx6q-spdif";
> + reg = <0x02004000 0x4000>;
> + interrupts = <0 52 0x04>;
> + dmas = <&sdma 14 18 0>,
> +<&sdma 15 18 0>;
> + dma-names = "rx", "tx";
> +
> + clocks = <&clks 197>;
> + clock-names = "core";
> + rx-clksrc-lock;
> + rx-clksrc-names =
> + "lock.ext", "lock.spdif", "lock.asrc",
> + "lock.spdif_ext", "lock.esai", "ext",
> + "spdif", "asrc", "spdif_ext", "esai",
> + "lock.mlb", "lock.mlb_phy", "mlb",
> + "mlb_phy";
> + tx-clksrc-names =
> + "xtal", "spdif", "asrc", "spdif_ext",
> + "esai", "ipg", "mlb", "mlb_phy";
I had a hard time understanding what you are doing here.
With this the clk names in arch/arm/mach-imx/clk-imx6q.c become an API
between the Kernel and the devicetree. Don't do that.
There is a standardized devicetree binding for clocks. Use it.
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [alsa-devel] [PATCH v4 1/2] ASoC: fsl: Add S/PDIF CPU DAI driver
On Wed, Aug 14, 2013 at 02:34:45PM +0800, Nicolin Chen wrote: > On Wed, Aug 14, 2013 at 02:39:33PM +0800, Shawn Guo wrote: > > We only need to maintain those versions that require different > > programming model in the list. For example, if S/PDIF on Vybrid > > is completely compatible with imx6q one and uses the exactly same > > programming model, we do not need to maintain a compatible string > > for Vybrid S/PDIF at all. Instead, we only need to have something > > like below in Vybrid dts file, and S/PDIF driver will just work for it. > > > > compatible = "fsl,vf600-spdif", "fsl,imx6q-spdif"; > > > > Shawn > > Clear. Thank you for the explain. > > Then I think I can merely remain "fsl,imx6q-spdif" here, > because all other cases should be completely compatible > with this one. They are only different in the clock source > names list, which's already being specified in dts file. > > Please correct me if you think this still isn't proper. And we generally prefer to use the soc that firstly integrates the IP to name the compatible. For IMX series, I think imx35 is the one, so we would name it "fsl,imx35-spdif". Shawn ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [GIT PULL] DT/core: cpu_ofnode updates for v3.12
On 13/08/13 19:37, Michal Simek wrote: > On 08/13/2013 05:40 PM, Sudeep KarkadaNagesha wrote: >> Adding PowerPC list >> >> On 13/08/13 14:00, Rafael J. Wysocki wrote: >>> On Monday, August 12, 2013 02:27:47 PM Sudeep KarkadaNagesha wrote: The following changes since commit d4e4ab86bcba5a72779c43dc1459f71fea3d89c8: Linux 3.11-rc5 (2013-08-11 18:04:20 -0700) are available in the git repository at: git://linux-arm.org/linux-skn.git cpu_of_node for you to fetch changes up to 9e9e26dde91f22635c87d0e45f3938b5ded96f0d: cpufreq: pmac32-cpufreq: remove device tree parsing for cpu nodes (2013-08-12 10:22:29 +0100) Sudeep KarkadaNagesha (16): of: add support for retrieving cpu node for a given logical cpu index ARM: DT/kernel: define ARM specific arch_match_cpu_phys_id driver/core: cpu: initialize of_node in cpu's device struture of/device: add helper to get cpu device node from logical cpu index ARM: topology: remove hwid/MPIDR dependency from cpu_capacity ARM: mvebu: remove device tree parsing for cpu nodes drivers/bus: arm-cci: avoid parsing DT for cpu device nodes cpufreq: imx6q-cpufreq: remove device tree parsing for cpu nodes cpufreq: cpufreq-cpu0: remove device tree parsing for cpu nodes cpufreq: highbank-cpufreq: remove device tree parsing for cpu nodes cpufreq: spear-cpufreq: remove device tree parsing for cpu nodes cpufreq: kirkwood-cpufreq: remove device tree parsing for cpu nodes cpufreq: arm_big_little: remove device tree parsing for cpu nodes cpufreq: maple-cpufreq: remove device tree parsing for cpu nodes cpufreq: pmac64-cpufreq: remove device tree parsing for cpu nodes cpufreq: pmac32-cpufreq: remove device tree parsing for cpu nodes arch/arm/kernel/devtree.c | 5 + arch/arm/kernel/topology.c | 61 +++-- arch/arm/mach-imx/mach-imx6q.c | 3 +-- arch/arm/mach-mvebu/platsmp.c | 52 drivers/base/cpu.c | 2 ++ drivers/bus/arm-cci.c | 28 +++- drivers/cpufreq/arm_big_little_dt.c | 40 ++-- drivers/cpufreq/cpufreq-cpu0.c | 23 --- drivers/cpufreq/highbank-cpufreq.c | 18 ++ drivers/cpufreq/imx6q-cpufreq.c | 4 +--- drivers/cpufreq/kirkwood-cpufreq.c | 8 +--- drivers/cpufreq/maple-cpufreq.c | 23 +++ drivers/cpufreq/pmac32-cpufreq.c| 5 +++-- drivers/cpufreq/pmac64-cpufreq.c| 47 +++ drivers/cpufreq/spear-cpufreq.c | 4 ++-- drivers/of/base.c | 73 + >> include/linux/cpu.h | 1 + include/linux/of.h | 6 ++ include/linux/of_device.h | 15 +++ 19 files changed, 202 insertions(+), 216 deletions(-) PS: This patch series is reviewed and acknowledged @ v1: https://lkml.org/lkml/2013/7/15/128 v2: https://lkml.org/lkml/2013/7/17/341 v3: https://lkml.org/lkml/2013/7/22/219 >>> >>> Pulled, thanks! >>> >> Hi Rob, Rafael, >> >> On 13/08/13 15:16, kbuild test robot wrote:> tree: >> git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git >> bleeding-edge >>> head: 0d4bcb5dc7d3040c0ce7572ea30ab9e5d9455bfa commit: >>> 7939ff8d991de2c0b15064e76ee549a6df5ae67f [151/204] of: add >>> support for retrieving cpu node for a given logical cpu index >>> config: make ARCH=powerpc allmodconfig >>> >>> All error/warnings: >>> >>> warning: (MPC836x_RDK && MTD_NAND_FSL_ELBC && MTD_NAND_FSL_UPM) >>> selects FSL_LBC which has unmet direct dependencies (FSL_SOC) >>> warning: (MPC836x_RDK && MTD_NAND_FSL_ELBC && MTD_NAND_FSL_UPM) >>> selects FSL_LBC which has unmet direct dependencies (FSL_SOC) >>> In file included from arch/powerpc/include/asm/kvm_para.h:26:0, from >>> include/uapi/linux/kvm_para.h:26, from include/linux/kvm_para.h:4, >>> from include/linux/kvm_host.h:30, from >>> arch/powerpc/kernel/asm-offsets.c:53: >>> include/linux/of.h:269:28: error: conflicting types for >>> 'of_get_cpu_node' >>> extern struct device_node *of_get_cpu_node(int cpu); ^ In file >>> included from include/linux/of.h:139:0, from >>> arch/powerpc/include/asm/kvm_para.h:26, from >>> include/uapi/linux/kvm_para.h:26, from include/linux/kvm_para.h:4, >>> from include/linux/kvm_host.h:30, from >>> arch/powerpc/kernel/asm-offsets.c:53: >>> arch/powerpc/include/asm/prom.h:47:21: note: previous declaration >>> of 'of_get_cpu_node' was here >>> struct device_node *of_get_c
Re: [PATCH v4 resent 1/2] ASoC: fsl: Add S/PDIF CPU DAI driver
Hi Sascha,
On Wed, Aug 14, 2013 at 09:50:17AM +0200, Sascha Hauer wrote:
> > + - tx-clksrc-names : The names for all available clock sources for tx,
> > which
> > + is also being listed in SoC reference manual, ClkSrc_Sel bit of
> > SPDIF_SRPC.
> > + And the name list would be different between different SoC. Use 'null'
> > for
> > + those unlisted names, and the max number of tx-clksrc-names should be 8.
> > +
> > + - rx-clksrc-names : The names for all available clock sources for rx,
> > which
> > + is also being listed in SoC reference manual, TxClk_Source bit of
> > SPDIF_STC.
> > + And the name list would be different between different SoC. Use 'null'
> > for
> > + those unlisted names, and the max number of rx-clksrc-names should be 16.
> > +
> > +Optional properties:
> > +
> > + - rx-clksrc-lock: This is a boolean property. If present, ClkSrc_Sel bit
> > + of SPDIF_SRPC would be set a clock source that cares DPLL locked
> > condition.
> > +
> > +Example1:
> > +
> > +spdif: spdif@02004000 {
> > + clocks = <&clks 197>;
> > + clock-names = "core";
> > + rx-clksrc-lock;
> > + rx-clksrc-names =
> > + "lock.ext", "lock.spdif", "lock.asrc",
> > + "lock.spdif_ext", "lock.esai", "ext",
> > + "spdif", "asrc", "spdif_ext", "esai",
> > + "lock.mlb", "lock.mlb_phy", "mlb",
> > + "mlb_phy";
> > + tx-clksrc-names =
> > + "xtal", "spdif", "asrc", "spdif_ext",
> > + "esai", "ipg", "mlb", "mlb_phy";
>
> I had a hard time understanding what you are doing here.
>
> With this the clk names in arch/arm/mach-imx/clk-imx6q.c become an API
> between the Kernel and the devicetree. Don't do that.
>
> There is a standardized devicetree binding for clocks. Use it.
I think I should first explain to you what this part is doing:
The driver needs to set Clk_source bit for TX/RX to select the
clock from a clock mux. The names listed above are those of the
clocks connecting to the mux, while they are not only internal
clocks which're included in clk-imx6q.c but also external ones,
an on-board external osc for example.
The driver does get the clock by using the standard DT binding,
see the 'clocks = <&clks 197>' above, and then compare this
obtained clock->name with the name list to decide which value
should be set to the Clk_source bit.
==
ClkSrc_Sel from i.MX6Q reference manual:
Clock source selection, all other settings not shown are reserved:
if (DPLL Locked) SPDIF_RxClk else extal
0001 if (DPLL Locked) SPDIF_RxClk else spdif_clk
0010 if (DPLL Locked) SPDIF_RxClk else asrc_clk
0011 if (DPLL Locked) SPDIF_RxClk else spdif_extclk
0100 if (DPLL Locked) SPDIF_Rxclk else esai_hckt
0101 extal_clk
0110 spdif_clk
0111 asrc_clk
1000 spdif_extclk
1001 esai_hckt
1010 if (DPLL Locked) SPDIF_RxClk else mlb_clk
==
So the name list here basically is not being used to obtain a
clock like what standardized DT binding does but to provide the
driver a full list to look up which value should be exactly used
according to the obtained clock.
I think I should revise the binding doc for these two lists. It
might be hard to explain within that kinda short paragraph.
Surely, if I misunderstand your point, please correct me. And
if you have any sage idea, please guide me.
Thank you,
Nicolin Chen
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH] Revert "cxgb3: Check and handle the dma mapping errors"
This reverts commit f83331bab149e29fa2c49cf102c0cd8c3f1ce9f9.
As the tests PPC64 (powernv platform) show, IOMMU pages are leaking
when transferring big amount of small packets (<=64 bytes),
"ping -f" and waiting for 15 seconds is the simplest way to confirm the bug.
Cc: Linus Torvalds
Cc: Santosh Rastapur
Cc: Jay Fenlason
Cc: David S. Miller
Cc: Divy Le ray
Signed-off-by: Alexey Kardashevskiy
---
drivers/net/ethernet/chelsio/cxgb3/sge.c | 107 +++
1 file changed, 24 insertions(+), 83 deletions(-)
diff --git a/drivers/net/ethernet/chelsio/cxgb3/sge.c
b/drivers/net/ethernet/chelsio/cxgb3/sge.c
index 687ec4a..9c89dc8 100644
--- a/drivers/net/ethernet/chelsio/cxgb3/sge.c
+++ b/drivers/net/ethernet/chelsio/cxgb3/sge.c
@@ -455,11 +455,6 @@ static int alloc_pg_chunk(struct adapter *adapter, struct
sge_fl *q,
q->pg_chunk.offset = 0;
mapping = pci_map_page(adapter->pdev, q->pg_chunk.page,
0, q->alloc_size, PCI_DMA_FROMDEVICE);
- if (unlikely(pci_dma_mapping_error(adapter->pdev, mapping))) {
- __free_pages(q->pg_chunk.page, order);
- q->pg_chunk.page = NULL;
- return -EIO;
- }
q->pg_chunk.mapping = mapping;
}
sd->pg_chunk = q->pg_chunk;
@@ -954,75 +949,40 @@ static inline unsigned int calc_tx_descs(const struct
sk_buff *skb)
return flits_to_desc(flits);
}
-
-/* map_skb - map a packet main body and its page fragments
- * @pdev: the PCI device
- * @skb: the packet
- * @addr: placeholder to save the mapped addresses
- *
- * map the main body of an sk_buff and its page fragments, if any.
- */
-static int map_skb(struct pci_dev *pdev, const struct sk_buff *skb,
- dma_addr_t *addr)
-{
- const skb_frag_t *fp, *end;
- const struct skb_shared_info *si;
-
- *addr = pci_map_single(pdev, skb->data, skb_headlen(skb),
- PCI_DMA_TODEVICE);
- if (pci_dma_mapping_error(pdev, *addr))
- goto out_err;
-
- si = skb_shinfo(skb);
- end = &si->frags[si->nr_frags];
-
- for (fp = si->frags; fp < end; fp++) {
- *++addr = skb_frag_dma_map(&pdev->dev, fp, 0, skb_frag_size(fp),
- DMA_TO_DEVICE);
- if (pci_dma_mapping_error(pdev, *addr))
- goto unwind;
- }
- return 0;
-
-unwind:
- while (fp-- > si->frags)
- dma_unmap_page(&pdev->dev, *--addr, skb_frag_size(fp),
- DMA_TO_DEVICE);
-
- pci_unmap_single(pdev, addr[-1], skb_headlen(skb), PCI_DMA_TODEVICE);
-out_err:
- return -ENOMEM;
-}
-
/**
- * write_sgl - populate a scatter/gather list for a packet
+ * make_sgl - populate a scatter/gather list for a packet
* @skb: the packet
* @sgp: the SGL to populate
* @start: start address of skb main body data to include in the SGL
* @len: length of skb main body data to include in the SGL
- * @addr: the list of the mapped addresses
+ * @pdev: the PCI device
*
- * Copies the scatter/gather list for the buffers that make up a packet
+ * Generates a scatter/gather list for the buffers that make up a packet
* and returns the SGL size in 8-byte words. The caller must size the SGL
* appropriately.
*/
-static inline unsigned int write_sgl(const struct sk_buff *skb,
+static inline unsigned int make_sgl(const struct sk_buff *skb,
struct sg_ent *sgp, unsigned char *start,
- unsigned int len, const dma_addr_t *addr)
+ unsigned int len, struct pci_dev *pdev)
{
- unsigned int i, j = 0, k = 0, nfrags;
+ dma_addr_t mapping;
+ unsigned int i, j = 0, nfrags;
if (len) {
+ mapping = pci_map_single(pdev, start, len, PCI_DMA_TODEVICE);
sgp->len[0] = cpu_to_be32(len);
- sgp->addr[j++] = cpu_to_be64(addr[k++]);
+ sgp->addr[0] = cpu_to_be64(mapping);
+ j = 1;
}
nfrags = skb_shinfo(skb)->nr_frags;
for (i = 0; i < nfrags; i++) {
const skb_frag_t *frag = &skb_shinfo(skb)->frags[i];
+ mapping = skb_frag_dma_map(&pdev->dev, frag, 0,
skb_frag_size(frag),
+ DMA_TO_DEVICE);
sgp->len[j] = cpu_to_be32(skb_frag_size(frag));
- sgp->addr[j] = cpu_to_be64(addr[k++]);
+ sgp->addr[j] = cpu_to_be64(mapping);
j ^= 1;
if (j == 0)
++sgp;
@@ -1178,7 +1138,7 @@ static void write_tx_pkt_wr(struct adapter *adap, struct
sk_buff *skb,
const struct port_info *pi,
unsi
Re: [GIT PULL] DT/core: cpu_ofnode updates for v3.12
On 13/08/13 20:45, Rafael J. Wysocki wrote: > On Tuesday, August 13, 2013 01:44:23 PM Rob Herring wrote: >> On Tue, Aug 13, 2013 at 10:40 AM, Sudeep KarkadaNagesha >> wrote: >>> Adding PowerPC list >>> >>> On 13/08/13 14:00, Rafael J. Wysocki wrote: On Monday, August 12, 2013 02:27:47 PM Sudeep KarkadaNagesha wrote: > The following changes since commit > d4e4ab86bcba5a72779c43dc1459f71fea3d89c8: > > Linux 3.11-rc5 (2013-08-11 18:04:20 -0700) > > are available in the git repository at: > > git://linux-arm.org/linux-skn.git cpu_of_node >> >> [snip] >> All error/warnings: warning: (MPC836x_RDK && MTD_NAND_FSL_ELBC && MTD_NAND_FSL_UPM) selects FSL_LBC which has unmet direct dependencies (FSL_SOC) warning: (MPC836x_RDK && MTD_NAND_FSL_ELBC && MTD_NAND_FSL_UPM) selects FSL_LBC which has unmet direct dependencies (FSL_SOC) In file included from arch/powerpc/include/asm/kvm_para.h:26:0, from include/uapi/linux/kvm_para.h:26, from include/linux/kvm_para.h:4, from include/linux/kvm_host.h:30, from arch/powerpc/kernel/asm-offsets.c:53: include/linux/of.h:269:28: error: conflicting types for 'of_get_cpu_node' extern struct device_node *of_get_cpu_node(int cpu); ^ In file included from include/linux/of.h:139:0, from arch/powerpc/include/asm/kvm_para.h:26, from include/uapi/linux/kvm_para.h:26, from include/linux/kvm_para.h:4, from include/linux/kvm_host.h:30, from arch/powerpc/kernel/asm-offsets.c:53: arch/powerpc/include/asm/prom.h:47:21: note: previous declaration of 'of_get_cpu_node' was here struct device_node *of_get_cpu_node(int cpu, unsigned int *thread); ^ make[2]: *** [arch/powerpc/kernel/asm-offsets.s] Error 1 make[2]: Target `__build' not remade because of errors. make[1]: *** [prepare0] Error 2 make[1]: Target `prepare' not remade because of errors. make: *** [sub-make] Error 2 >>> >>> There seems to be conflict in the new function "of_get_cpu_node" added. >>> PowerPC also defines the same function name. Further microblaze and >>> openrisc declares it(can be removed) but doesn't define it. >>> To fix this: >>> 1. I can rename the newly added function to something different like >>>`of_get_cpunode` or >>> 2. If of_* namespace should be used by only OF/FDT and not by any >>>architecture specific code, then the arch specific version can be >>>renamed to some thing like arch_of_get_cpu_node. >>>Also most of the calls to arch specific function can be moved to >>>generic code. >>> >>> Let me know your thoughts. >> >> It is up to Rafael if he is willing/able to rebase his tree, but I >> would drop this series until this is sorted out. > > Yeah, I've just done that. > Thanks Rafael, sorry for the trouble. I didn't expect of_* name-space to be used in ARCH specific code. >> I think the new common function should be and can be generalized to work for >> powerpc. >> It would need to make reg property optional and pass in the device >> node to the arch specific function. >> >> A short term solution would be just to make the function "#ifndef >> CONFIG_PPC". > > I wouldn't do that, it's almost guaranteed to be messy going forward. > > I'd go for 1 above personally. Even though it's easier approach, I would go for fixing PPC and converge at generic of_get_cpu_node implementation if possible. Regards, Sudeep ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH] KVM: PPC: POWERNV: move iommu_add_device earlier
The current implementation of IOMMU on sPAPR does not use iommu_ops
and therefore does not call IOMMU API's bus_set_iommu() which
1) sets iommu_ops for a bus
2) registers a bus notifier
Instead, PCI devices are added to IOMMU groups from
subsys_initcall_sync(tce_iommu_init) which does basically the same
thing without using iommu_ops callbacks.
However Freescale PAMU driver (https://lkml.org/lkml/2013/7/1/158)
implements iommu_ops and when tce_iommu_init is called, every PCI device
is already added to some group so there is a conflict.
This patch does 2 things:
1. removes the loop in which PCI devices were added to groups and
adds explicit iommu_add_device() calls to add devices as soon as they get
the iommu_table pointer assigned to them.
2. moves a bus notifier to powernv code in order to avoid conflict with
the notifier from Freescale driver.
iommu_add_device() and iommu_del_device() are public now.
Signed-off-by: Alexey Kardashevskiy
---
arch/powerpc/include/asm/iommu.h| 2 ++
arch/powerpc/kernel/iommu.c | 41 +++--
arch/powerpc/platforms/powernv/pci-ioda.c | 12 ++---
arch/powerpc/platforms/powernv/pci-p5ioc2.c | 1 +
arch/powerpc/platforms/powernv/pci.c| 31 ++
arch/powerpc/platforms/pseries/iommu.c | 7 +++--
6 files changed, 51 insertions(+), 43 deletions(-)
diff --git a/arch/powerpc/include/asm/iommu.h b/arch/powerpc/include/asm/iommu.h
index c34656a..ba74329 100644
--- a/arch/powerpc/include/asm/iommu.h
+++ b/arch/powerpc/include/asm/iommu.h
@@ -103,6 +103,8 @@ extern struct iommu_table *iommu_init_table(struct
iommu_table * tbl,
int nid);
extern void iommu_register_group(struct iommu_table *tbl,
int pci_domain_number, unsigned long pe_num);
+extern int iommu_add_device(struct device *dev);
+extern void iommu_del_device(struct device *dev);
extern int iommu_map_sg(struct device *dev, struct iommu_table *tbl,
struct scatterlist *sglist, int nelems,
diff --git a/arch/powerpc/kernel/iommu.c b/arch/powerpc/kernel/iommu.c
index b20ff17..15f8ca8 100644
--- a/arch/powerpc/kernel/iommu.c
+++ b/arch/powerpc/kernel/iommu.c
@@ -1105,7 +1105,7 @@ void iommu_release_ownership(struct iommu_table *tbl)
}
EXPORT_SYMBOL_GPL(iommu_release_ownership);
-static int iommu_add_device(struct device *dev)
+int iommu_add_device(struct device *dev)
{
struct iommu_table *tbl;
int ret = 0;
@@ -1134,46 +1134,13 @@ static int iommu_add_device(struct device *dev)
return ret;
}
+EXPORT_SYMBOL_GPL(iommu_add_device);
-static void iommu_del_device(struct device *dev)
+void iommu_del_device(struct device *dev)
{
iommu_group_remove_device(dev);
}
-
-static int iommu_bus_notifier(struct notifier_block *nb,
- unsigned long action, void *data)
-{
- struct device *dev = data;
-
- switch (action) {
- case BUS_NOTIFY_ADD_DEVICE:
- return iommu_add_device(dev);
- case BUS_NOTIFY_DEL_DEVICE:
- iommu_del_device(dev);
- return 0;
- default:
- return 0;
- }
-}
-
-static struct notifier_block tce_iommu_bus_nb = {
- .notifier_call = iommu_bus_notifier,
-};
-
-static int __init tce_iommu_init(void)
-{
- struct pci_dev *pdev = NULL;
-
- BUILD_BUG_ON(PAGE_SIZE < IOMMU_PAGE_SIZE);
-
- for_each_pci_dev(pdev)
- iommu_add_device(&pdev->dev);
-
- bus_register_notifier(&pci_bus_type, &tce_iommu_bus_nb);
- return 0;
-}
-
-subsys_initcall_sync(tce_iommu_init);
+EXPORT_SYMBOL_GPL(iommu_del_device);
#else
diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c
b/arch/powerpc/platforms/powernv/pci-ioda.c
index d8140b1..a9f8fef 100644
--- a/arch/powerpc/platforms/powernv/pci-ioda.c
+++ b/arch/powerpc/platforms/powernv/pci-ioda.c
@@ -441,6 +441,7 @@ static void pnv_pci_ioda_dma_dev_setup(struct pnv_phb *phb,
struct pci_dev *pdev
pe = &phb->ioda.pe_array[pdn->pe_number];
set_iommu_table_base(&pdev->dev, &pe->tce32_table);
+ iommu_add_device(&pdev->dev);
}
static void pnv_ioda_setup_bus_dma(struct pnv_ioda_pe *pe, struct pci_bus *bus)
@@ -449,6 +450,7 @@ static void pnv_ioda_setup_bus_dma(struct pnv_ioda_pe *pe,
struct pci_bus *bus)
list_for_each_entry(dev, &bus->devices, bus_list) {
set_iommu_table_base(&dev->dev, &pe->tce32_table);
+ iommu_add_device(&dev->dev);
if (dev->subordinate)
pnv_ioda_setup_bus_dma(pe, dev->subordinate);
}
@@ -610,9 +612,10 @@ static void pnv_pci_ioda_setup_dma_pe(struct pnv_phb *phb,
iommu_init_table(tbl, phb->hose->node);
iommu_register_group(tbl, pci_domain_nr(pe->pbus), pe->pe_number);
- if (pe->pdev)
+ if (pe->pdev) {
set_iommu_table_base(&pe->pdev->dev,
Re: [alsa-devel] [PATCH v4 resent 1/2] ASoC: fsl: Add S/PDIF CPU DAI driver
On Wed, Aug 14, 2013 at 04:48:02PM +0800, Nicolin Chen wrote:
> Hi Sascha,
>
> On Wed, Aug 14, 2013 at 09:50:17AM +0200, Sascha Hauer wrote:
> > > + - tx-clksrc-names : The names for all available clock sources for tx,
> > > which
> > > + is also being listed in SoC reference manual, ClkSrc_Sel bit of
> > > SPDIF_SRPC.
> > > + And the name list would be different between different SoC. Use 'null'
> > > for
> > > + those unlisted names, and the max number of tx-clksrc-names should be
> > > 8.
> > > +
> > > + - rx-clksrc-names : The names for all available clock sources for rx,
> > > which
> > > + is also being listed in SoC reference manual, TxClk_Source bit of
> > > SPDIF_STC.
> > > + And the name list would be different between different SoC. Use 'null'
> > > for
> > > + those unlisted names, and the max number of rx-clksrc-names should be
> > > 16.
> > > +
> > > +Optional properties:
> > > +
> > > + - rx-clksrc-lock: This is a boolean property. If present, ClkSrc_Sel
> > > bit
> > > + of SPDIF_SRPC would be set a clock source that cares DPLL locked
> > > condition.
> > > +
> > > +Example1:
> > > +
> > > +spdif: spdif@02004000 {
> > > + clocks = <&clks 197>;
> > > + clock-names = "core";
> > > + rx-clksrc-lock;
> > > + rx-clksrc-names =
> > > + "lock.ext", "lock.spdif", "lock.asrc",
> > > + "lock.spdif_ext", "lock.esai", "ext",
> > > + "spdif", "asrc", "spdif_ext", "esai",
> > > + "lock.mlb", "lock.mlb_phy", "mlb",
> > > + "mlb_phy";
> > > + tx-clksrc-names =
> > > + "xtal", "spdif", "asrc", "spdif_ext",
> > > + "esai", "ipg", "mlb", "mlb_phy";
> >
> > I had a hard time understanding what you are doing here.
> >
> > With this the clk names in arch/arm/mach-imx/clk-imx6q.c become an API
> > between the Kernel and the devicetree. Don't do that.
> >
> > There is a standardized devicetree binding for clocks. Use it.
>
> I think I should first explain to you what this part is doing:
> The driver needs to set Clk_source bit for TX/RX to select the
> clock from a clock mux. The names listed above are those of the
> clocks connecting to the mux, while they are not only internal
> clocks which're included in clk-imx6q.c but also external ones,
> an on-board external osc for example.
>
> The driver does get the clock by using the standard DT binding,
> see the 'clocks = <&clks 197>' above, and then compare this
> obtained clock->name with the name list to decide which value
> should be set to the Clk_source bit.
>
> ==
> ClkSrc_Sel from i.MX6Q reference manual:
>
> Clock source selection, all other settings not shown are reserved:
> if (DPLL Locked) SPDIF_RxClk else extal
> 0001 if (DPLL Locked) SPDIF_RxClk else spdif_clk
> 0010 if (DPLL Locked) SPDIF_RxClk else asrc_clk
> 0011 if (DPLL Locked) SPDIF_RxClk else spdif_extclk
> 0100 if (DPLL Locked) SPDIF_Rxclk else esai_hckt
> 0101 extal_clk
> 0110 spdif_clk
> 0111 asrc_clk
> 1000 spdif_extclk
> 1001 esai_hckt
> 1010 if (DPLL Locked) SPDIF_RxClk else mlb_clk
> ==
>
> So the name list here basically is not being used to obtain a
> clock like what standardized DT binding does but to provide the
> driver a full list to look up which value should be exactly used
> according to the obtained clock.
>
> I think I should revise the binding doc for these two lists. It
> might be hard to explain within that kinda short paragraph.
>
> Surely, if I misunderstand your point, please correct me. And
> if you have any sage idea, please guide me.
Something like this:
clocks = <&clks 197>, <&clks 3>, <&clks 197>, <&clks 107>, <&clks SPDIF_EXT>,
<&clks 118>, <&clks 62>, <&clks 139>, <&clks MLB_PHY>
clock-names = "core", "rxtx0", "rxtx1", "rxtx2", "rxtx3", "rxtx4", "rxtx5",
"rxtx6", "rxtx7"
This describes the different input clocks to the spdif core and also
gives a hint to the array index (rxtx_n_) to use.
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH] KVM: PPC: POWERNV: move iommu_add_device earlier
> -Original Message-
> From: Linuxppc-dev [mailto:linuxppc-dev-
> bounces+bharat.bhushan=freescale@lists.ozlabs.org] On Behalf Of Alexey
> Kardashevskiy
> Sent: Wednesday, August 14, 2013 2:55 PM
> To: linuxppc-dev@lists.ozlabs.org
> Cc: Alexey Kardashevskiy; Paul Mackerras; linux-ker...@vger.kernel.org
> Subject: [PATCH] KVM: PPC: POWERNV: move iommu_add_device earlier
>
> The current implementation of IOMMU on sPAPR does not use iommu_ops
> and therefore does not call IOMMU API's bus_set_iommu() which
> 1) sets iommu_ops for a bus
> 2) registers a bus notifier
> Instead, PCI devices are added to IOMMU groups from
> subsys_initcall_sync(tce_iommu_init) which does basically the same
> thing without using iommu_ops callbacks.
>
> However Freescale PAMU driver (https://lkml.org/lkml/2013/7/1/158)
> implements iommu_ops and when tce_iommu_init is called, every PCI device
> is already added to some group so there is a conflict.
>
> This patch does 2 things:
> 1. removes the loop in which PCI devices were added to groups and
> adds explicit iommu_add_device() calls to add devices as soon as they get
> the iommu_table pointer assigned to them.
> 2. moves a bus notifier to powernv code in order to avoid conflict with
> the notifier from Freescale driver.
>
> iommu_add_device() and iommu_del_device() are public now.
This works for me (able to boot Linux, as expected) :-)
But a question, why not move arch/powerpc/kernel/iommu.c in platform/ ? or use
this for book3s or not_book3e only?
Thanks
-Bharat
>
> Signed-off-by: Alexey Kardashevskiy
> ---
> arch/powerpc/include/asm/iommu.h| 2 ++
> arch/powerpc/kernel/iommu.c | 41
> +++--
> arch/powerpc/platforms/powernv/pci-ioda.c | 12 ++---
> arch/powerpc/platforms/powernv/pci-p5ioc2.c | 1 +
> arch/powerpc/platforms/powernv/pci.c| 31 ++
> arch/powerpc/platforms/pseries/iommu.c | 7 +++--
> 6 files changed, 51 insertions(+), 43 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/iommu.h
> b/arch/powerpc/include/asm/iommu.h
> index c34656a..ba74329 100644
> --- a/arch/powerpc/include/asm/iommu.h
> +++ b/arch/powerpc/include/asm/iommu.h
> @@ -103,6 +103,8 @@ extern struct iommu_table *iommu_init_table(struct
> iommu_table * tbl,
> int nid);
> extern void iommu_register_group(struct iommu_table *tbl,
>int pci_domain_number, unsigned long pe_num);
> +extern int iommu_add_device(struct device *dev);
> +extern void iommu_del_device(struct device *dev);
>
> extern int iommu_map_sg(struct device *dev, struct iommu_table *tbl,
> struct scatterlist *sglist, int nelems,
> diff --git a/arch/powerpc/kernel/iommu.c b/arch/powerpc/kernel/iommu.c
> index b20ff17..15f8ca8 100644
> --- a/arch/powerpc/kernel/iommu.c
> +++ b/arch/powerpc/kernel/iommu.c
> @@ -1105,7 +1105,7 @@ void iommu_release_ownership(struct iommu_table *tbl)
> }
> EXPORT_SYMBOL_GPL(iommu_release_ownership);
>
> -static int iommu_add_device(struct device *dev)
> +int iommu_add_device(struct device *dev)
> {
> struct iommu_table *tbl;
> int ret = 0;
> @@ -1134,46 +1134,13 @@ static int iommu_add_device(struct device *dev)
>
> return ret;
> }
> +EXPORT_SYMBOL_GPL(iommu_add_device);
>
> -static void iommu_del_device(struct device *dev)
> +void iommu_del_device(struct device *dev)
> {
> iommu_group_remove_device(dev);
> }
> -
> -static int iommu_bus_notifier(struct notifier_block *nb,
> - unsigned long action, void *data)
> -{
> - struct device *dev = data;
> -
> - switch (action) {
> - case BUS_NOTIFY_ADD_DEVICE:
> - return iommu_add_device(dev);
> - case BUS_NOTIFY_DEL_DEVICE:
> - iommu_del_device(dev);
> - return 0;
> - default:
> - return 0;
> - }
> -}
> -
> -static struct notifier_block tce_iommu_bus_nb = {
> - .notifier_call = iommu_bus_notifier,
> -};
> -
> -static int __init tce_iommu_init(void)
> -{
> - struct pci_dev *pdev = NULL;
> -
> - BUILD_BUG_ON(PAGE_SIZE < IOMMU_PAGE_SIZE);
> -
> - for_each_pci_dev(pdev)
> - iommu_add_device(&pdev->dev);
> -
> - bus_register_notifier(&pci_bus_type, &tce_iommu_bus_nb);
> - return 0;
> -}
> -
> -subsys_initcall_sync(tce_iommu_init);
> +EXPORT_SYMBOL_GPL(iommu_del_device);
>
> #else
>
> diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c
> b/arch/powerpc/platforms/powernv/pci-ioda.c
> index d8140b1..a9f8fef 100644
> --- a/arch/powerpc/platforms/powernv/pci-ioda.c
> +++ b/arch/powerpc/platforms/powernv/pci-ioda.c
> @@ -441,6 +441,7 @@ static void pnv_pci_ioda_dma_dev_setup(struct pnv_phb
> *phb,
> struct pci_dev *pdev
>
> pe = &phb->ioda.pe_array[pdn->pe_number];
> set_iommu_table_base(&pdev->dev, &pe->tce32_table);
> + iommu_add_device(&pdev->dev);
> }
>
> static void
Re: [PATCH 1/3] powerpc: Add iommu domain pointer to device archdata
On Mon, Jul 15, 2013 at 10:20:55AM +0530, Varun Sethi wrote: > Add an iommu domain pointer to device (powerpc) archdata. Devices > are attached to iommu domains and this pointer provides a mechanism > to correlate between a device and the associated iommu domain. This > field is set when a device is attached to a domain. > > Signed-off-by: Varun Sethi > Acked-by: Kumar Gala > --- > - rebased patch to 3.11-rc1 > arch/powerpc/include/asm/device.h |3 +++ > 1 files changed, 3 insertions(+), 0 deletions(-) Okay, I applied these patches to my ppc/pamu branch. But before I merge it to my next branch (so that it can go upstream) I want to have a compile-test-case first. Can you send me a working .config which includes this driver? Thanks, Joerg ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [GIT PULL] DT/core: cpu_ofnode updates for v3.12
On 13/08/13 22:07, Benjamin Herrenschmidt wrote:
> On Tue, 2013-08-13 at 19:29 +0100, Sudeep KarkadaNagesha wrote:
>> I don't understand completely the use of ibm,ppc-interrupt-server#s and
>> its implications on generic of_get_cpu_node implementation.
>> I see the PPC specific definition of of_get_cpu_node uses thread id only
>> in 2 instances. Based on that, I have tried to move all the other
>> instances to use generic definition.
>>
>> Let me know if the idea is correct.
>
> No. The device-tree historically only represents cores, not HW threads, so
> it makes sense to retrieve also the thread number corresponding to the CPU.
>
Ok
> However, the mechanism to represent HW threads in the device-tree is currently
> somewhat platform specific (the ibm,ppc-interrupt-server#s).
I see most of the callers pass NULL to thread id argument except 2
instances in entire tree. If that's the case why can't we move to use
generic of_get_cpu_node in most of those cases and have PPC specific
implementation for the ones using thread id.
>
> So what you could do for now is:
>
> - Have a generic version that always returns 0 as the thread, which is weak
I would prefer to move to generic of_get_cpu_node where ever possible
and rename the function that takes thread id rather than making generic
one weak.
>
> - powerpc keeps its own implementation
How about only in cases where it needs thread_id.
>
> - Start a discussion on the bindings (if not already there) to define threads
> in a better way at which point the generic function can be updated.
>
I am not sure if we need to define any new bindings. Excerpts from ePAPR
(v1.1):
"3.7.1 General Properties of CPU nodes
The value of "reg" is a that defines a unique
CPU/thread id for the CPU/threads represented by the CPU node.
If a CPU supports more than one thread (i.e. multiple streams of
execution) the reg property is an array with 1 element per thread. The
#address-cells on the /cpus node specifies how many cells each element
of the array takes. Software can determine the number of threads by
dividing the size of reg by the parent node's #address-cells."
And this is exactly in agreement to what's implemented in the generic
of_get_cpu_node:
for_each_child_of_node(cpus, cpun) {
if (of_node_cmp(cpun->type, "cpu"))
continue;
cell = of_get_property(cpun, "reg", &prop_len);
if (!cell) {
pr_warn("%s: missing reg property\n", cpun->full_name);
continue;
}
prop_len /= sizeof(*cell);
while (prop_len) {
hwid = of_read_number(cell, ac);
prop_len -= ac;
if (arch_match_cpu_phys_id(cpu, hwid))
return cpun;
}
}
Yes this doesn't cover the historical "ibm,ppc-interrupt-server#s", for
which we can have PPC specific wrapper above the generic one i.e. get
the cpu node and then parse for thread id under custom property.
Let me know your thoughts.
Regards,
Sudeep
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [alsa-devel] [PATCH v4 resent 1/2] ASoC: fsl: Add S/PDIF CPU DAI driver
Hi, On Wed, Aug 14, 2013 at 11:56:52AM +0200, Sascha Hauer wrote: > > I think I should first explain to you what this part is doing: > > The driver needs to set Clk_source bit for TX/RX to select the > > clock from a clock mux. The names listed above are those of the > > clocks connecting to the mux, while they are not only internal > > clocks which're included in clk-imx6q.c but also external ones, > > an on-board external osc for example. > > > > The driver does get the clock by using the standard DT binding, > > see the 'clocks = <&clks 197>' above, and then compare this > > obtained clock->name with the name list to decide which value > > should be set to the Clk_source bit. > > > > == > > ClkSrc_Sel from i.MX6Q reference manual: > > > > Clock source selection, all other settings not shown are reserved: > > if (DPLL Locked) SPDIF_RxClk else extal > > 0001 if (DPLL Locked) SPDIF_RxClk else spdif_clk > > 0010 if (DPLL Locked) SPDIF_RxClk else asrc_clk > > 0011 if (DPLL Locked) SPDIF_RxClk else spdif_extclk > > 0100 if (DPLL Locked) SPDIF_Rxclk else esai_hckt > > 0101 extal_clk > > 0110 spdif_clk > > 0111 asrc_clk > > 1000 spdif_extclk > > 1001 esai_hckt > > 1010 if (DPLL Locked) SPDIF_RxClk else mlb_clk > > == > > > > So the name list here basically is not being used to obtain a > > clock like what standardized DT binding does but to provide the > > driver a full list to look up which value should be exactly used > > according to the obtained clock. > > > > I think I should revise the binding doc for these two lists. It > > might be hard to explain within that kinda short paragraph. > > > > Surely, if I misunderstand your point, please correct me. And > > if you have any sage idea, please guide me. > > Something like this: > > clocks = <&clks 197>, <&clks 3>, <&clks 197>, <&clks 107>, <&clks SPDIF_EXT>, > <&clks 118>, <&clks 62>, <&clks 139>, <&clks MLB_PHY> > clock-names = "core", "rxtx0", "rxtx1", "rxtx2", "rxtx3", "rxtx4", "rxtx5", > "rxtx6", "rxtx7" > > This describes the different input clocks to the spdif core and also > gives a hint to the array index (rxtx_n_) to use. Thank you for the idea, and..hmm..I'm a bit confused.. Is this really a nicer way? Actually the rx clock list and tx clock list are totally different. So doing this I have to list, in the maximum case, 24 (8 + 16) clock phandles for these two lists. And plussing another 6 I've listed in this binding doc -- thus there are totally 30 clock phanldes. But the 24 of 30 are only used to get two indexes. I think I need a little help here to understand why this is better. It looks more complicated to me. Creating the two name lists just because I can describe them in the dtsi of SoC, since they are totally fixed and identical to the SoC reference manual, while the clocks area can be remained for users to select the actual clocks (core/rx/tx/tx-32000/tx-44100/tx-48000), so they can configure these clocks in dts file of a specific board, and they don't need to touch the name lists any more. Thank you, Nicolin Chen ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] crypto: nx - fix concurrency issue
On Mon, Aug 12, 2013 at 06:49:37PM -0300, Marcelo Cerri wrote: > The NX driver uses the transformation context to store several fields > containing data related to the state of the operations in progress. > Since a single tfm can be used by different kernel threads at the same > time, we need to protect the data stored into the context. > > This patch makes use of spin locks to protect the data where a race > condition can happen. > > Reviewed-by: Fionnuala Gunter > Reviewed-by: Joy Latten > Signed-off-by: Marcelo Cerri Patch applied. Thanks a lot! -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [GIT PULL] DT/core: cpu_ofnode updates for v3.12
On Wed, 2013-08-14 at 11:01 +0100, Sudeep KarkadaNagesha wrote: > Yes this doesn't cover the historical "ibm,ppc-interrupt-server#s", > for > which we can have PPC specific wrapper above the generic one i.e. get > the cpu node and then parse for thread id under custom property. A wrapper is wrong. I don't want to have to have all ppc callers to use a different function. As I said, just make a generic one that returns a thread ID, ie, same signature as the powerpc one. Make it weak, we can override it in powerpc-land, or we can move the ibm,ppc-interrupt-server#s handling into the generic one, it won't hurt, but leave the thread_id return there, it doesn't hurt it will come in handy in a few cases without causing code duplication. Cheers, Ben. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[RFC V2 PATCH 0/6] cpuidle/ppc: Timer offload framework to support deep idle states
On PowerPC, when CPUs enter deep idle states, their local timers are switched off. The responsibility of waking them up at their next timer event, needs to be handed over to an external device. On PowerPC, we do not have an external device equivalent to HPET, which is currently done on architectures like x86. Instead we assign the local timer of one of the CPUs to do this job. This patchset is an attempt to make use of the existing timer broadcast framework in the kernel to meet the above requirement, except that the tick broadcast device is the local timer of the boot CPU. This patch series is ported ontop of 3.11-rc1 + the cpuidle driver backend for powernv posted by Deepthi Dharwar recently. NOHZ_FULL is disabled for all testing purposes. The current design and implementation supports the ONESHOT tick mode. It does not yet support the PERIODIC tick mode. The discussion around V1 of this patchset can be found here: https://lkml.org/lkml/2013/7/25/740. Changes since V1: 1. Dynamically pick a broadcast CPU, instead of having a dedicated one. 2. Remove the constraint of having to disable tickless idle on the broadcast CPU. Thanks to Ben H, Frederic Weisbecker, Li Yang and Vaidyanathan Srinivasan for all their comments and suggestions on the V1 of this patchset. Patch[1/6], Patch[2/6]: optimize the broadcast mechanism on ppc. Patch[3/6]: Introduces the core of the timer offload framework on powerpc. Patch[4/6]: Add a deep idle state to the cpuidle state table on powernv Patch[5/6]: Dynamically pick a broadcast CPU Patch[6/6]: Remove the constraint of having to disable tickless idle on the broadcast cpu, by queueing a hrtimer exclusively to do broadcast handling. --- Preeti U Murthy (4): cpuidle/ppc: Add timer offload framework to support deep idle states cpuidle/ppc: Add longnap state to the idle states on powernv cpuidle/ppc: Enable dynamic movement of the broadcast functionality across CPUs cpuidle/ppc : Queue a hrtimer on bc_cpu, explicitly to do broadcast handling Srivatsa S. Bhat (2): powerpc: Free up the IPI message slot of ipi call function (PPC_MSG_CALL_FUNC) powerpc: Implement broadcast timer interrupt as an IPI message arch/powerpc/include/asm/smp.h |3 - arch/powerpc/include/asm/time.h |9 ++ arch/powerpc/kernel/smp.c | 23 +++-- arch/powerpc/kernel/time.c | 114 +++ arch/powerpc/platforms/cell/interrupt.c |2 arch/powerpc/platforms/powernv/Kconfig |1 arch/powerpc/platforms/powernv/processor_idle.c | 104 + arch/powerpc/platforms/ps3/smp.c|2 8 files changed, 246 insertions(+), 12 deletions(-) ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[RFC V2 PATCH 2/6] powerpc: Implement broadcast timer interrupt as an IPI message
From: Srivatsa S. Bhat
For scalability and performance reasons, we want the broadcast timer
interrupts to be handled as efficiently as possible. Fixed IPI messages
are one of the most efficient mechanisms available - they are faster
than the smp_call_function mechanism because the IPI handlers are fixed
and hence they don't involve costly operations such as adding IPI handlers
to the target CPU's function queue, acquiring locks for synchronization etc.
Luckily we have an unused IPI message slot, so use that to implement
broadcast timer interrupts efficiently.
Signed-off-by: Srivatsa S. Bhat
Signed-off-by: Preeti U Murthy
---
arch/powerpc/include/asm/smp.h |3 ++-
arch/powerpc/kernel/smp.c | 19 +++
arch/powerpc/platforms/cell/interrupt.c |2 +-
arch/powerpc/platforms/ps3/smp.c|2 +-
4 files changed, 19 insertions(+), 7 deletions(-)
diff --git a/arch/powerpc/include/asm/smp.h b/arch/powerpc/include/asm/smp.h
index 51bf017..d877b69 100644
--- a/arch/powerpc/include/asm/smp.h
+++ b/arch/powerpc/include/asm/smp.h
@@ -117,7 +117,7 @@ extern int cpu_to_core_id(int cpu);
*
* Make sure this matches openpic_request_IPIs in open_pic.c, or what shows up
* in /proc/interrupts will be wrong!!! --Troy */
-#define PPC_MSG_UNUSED 0
+#define PPC_MSG_TIMER 0
#define PPC_MSG_RESCHEDULE 1
#define PPC_MSG_CALL_FUNC_SINGLE 2
#define PPC_MSG_DEBUGGER_BREAK 3
@@ -190,6 +190,7 @@ extern struct smp_ops_t *smp_ops;
extern void arch_send_call_function_single_ipi(int cpu);
extern void arch_send_call_function_ipi_mask(const struct cpumask *mask);
+extern void arch_send_tick_broadcast(const struct cpumask *mask);
/* Definitions relative to the secondary CPU spin loop
* and entry point. Not all of them exist on both 32 and
diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
index bc41e9f..6a68ca4 100644
--- a/arch/powerpc/kernel/smp.c
+++ b/arch/powerpc/kernel/smp.c
@@ -35,6 +35,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -111,9 +112,9 @@ int smp_generic_kick_cpu(int nr)
}
#endif /* CONFIG_PPC64 */
-static irqreturn_t unused_action(int irq, void *data)
+static irqreturn_t timer_action(int irq, void *data)
{
- /* This slot is unused and hence available for use, if needed */
+ timer_interrupt();
return IRQ_HANDLED;
}
@@ -144,14 +145,14 @@ static irqreturn_t debug_ipi_action(int irq, void *data)
}
static irq_handler_t smp_ipi_action[] = {
- [PPC_MSG_UNUSED] = unused_action, /* Slot available for future use */
+ [PPC_MSG_TIMER] = timer_action,
[PPC_MSG_RESCHEDULE] = reschedule_action,
[PPC_MSG_CALL_FUNC_SINGLE] = call_function_single_action,
[PPC_MSG_DEBUGGER_BREAK] = debug_ipi_action,
};
const char *smp_ipi_name[] = {
- [PPC_MSG_UNUSED] = "ipi unused",
+ [PPC_MSG_TIMER] = "ipi timer",
[PPC_MSG_RESCHEDULE] = "ipi reschedule",
[PPC_MSG_CALL_FUNC_SINGLE] = "ipi call function single",
[PPC_MSG_DEBUGGER_BREAK] = "ipi debugger",
@@ -221,6 +222,8 @@ irqreturn_t smp_ipi_demux(void)
all = xchg(&info->messages, 0);
#ifdef __BIG_ENDIAN
+ if (all & (1 << (24 - 8 * PPC_MSG_TIMER)))
+ timer_interrupt();
if (all & (1 << (24 - 8 * PPC_MSG_RESCHEDULE)))
scheduler_ipi();
if (all & (1 << (24 - 8 * PPC_MSG_CALL_FUNC_SINGLE)))
@@ -266,6 +269,14 @@ void arch_send_call_function_ipi_mask(const struct cpumask
*mask)
do_message_pass(cpu, PPC_MSG_CALL_FUNC_SINGLE);
}
+void arch_send_tick_broadcast(const struct cpumask *mask)
+{
+ unsigned int cpu;
+
+ for_each_cpu(cpu, mask)
+ do_message_pass(cpu, PPC_MSG_TIMER);
+}
+
#if defined(CONFIG_DEBUGGER) || defined(CONFIG_KEXEC)
void smp_send_debugger_break(void)
{
diff --git a/arch/powerpc/platforms/cell/interrupt.c
b/arch/powerpc/platforms/cell/interrupt.c
index 28166e4..1359113 100644
--- a/arch/powerpc/platforms/cell/interrupt.c
+++ b/arch/powerpc/platforms/cell/interrupt.c
@@ -213,7 +213,7 @@ static void iic_request_ipi(int msg)
void iic_request_IPIs(void)
{
- iic_request_ipi(PPC_MSG_UNUSED);
+ iic_request_ipi(PPC_MSG_TIMER);
iic_request_ipi(PPC_MSG_RESCHEDULE);
iic_request_ipi(PPC_MSG_CALL_FUNC_SINGLE);
iic_request_ipi(PPC_MSG_DEBUGGER_BREAK);
diff --git a/arch/powerpc/platforms/ps3/smp.c b/arch/powerpc/platforms/ps3/smp.c
index 488f069..5cb742a 100644
--- a/arch/powerpc/platforms/ps3/smp.c
+++ b/arch/powerpc/platforms/ps3/smp.c
@@ -74,7 +74,7 @@ static int __init ps3_smp_probe(void)
* to index needs to be setup.
*/
- BUILD_BUG_ON(PPC_MSG_UNUSED != 0);
+ BUILD_BUG_ON(PPC_MSG_TIMER!= 0);
BUILD_BUG_ON(PPC_MSG_RESCHEDULE != 1);
[RFC V2 PATCH 1/6] powerpc: Free up the IPI message slot of ipi call function (PPC_MSG_CALL_FUNC)
From: Srivatsa S. Bhat
The IPI handlers for both PPC_MSG_CALL_FUNC and PPC_MSG_CALL_FUNC_SINGLE
map to a common implementation - generic_smp_call_function_single_interrupt().
So, we can consolidate them and save one of the IPI message slots, (which are
precious, since only 4 of those slots are available).
So, implement the functionality of PPC_MSG_CALL_FUNC using
PPC_MSG_CALL_FUNC_SINGLE itself and release its IPI message slot, so that it
can be used for something else in the future, if desired.
Signed-off-by: Srivatsa S. Bhat
Signed-off-by: Preeti U Murthy
---
arch/powerpc/include/asm/smp.h |2 +-
arch/powerpc/kernel/smp.c | 12 +---
arch/powerpc/platforms/cell/interrupt.c |2 +-
arch/powerpc/platforms/ps3/smp.c|2 +-
4 files changed, 8 insertions(+), 10 deletions(-)
diff --git a/arch/powerpc/include/asm/smp.h b/arch/powerpc/include/asm/smp.h
index ffbaabe..51bf017 100644
--- a/arch/powerpc/include/asm/smp.h
+++ b/arch/powerpc/include/asm/smp.h
@@ -117,7 +117,7 @@ extern int cpu_to_core_id(int cpu);
*
* Make sure this matches openpic_request_IPIs in open_pic.c, or what shows up
* in /proc/interrupts will be wrong!!! --Troy */
-#define PPC_MSG_CALL_FUNCTION 0
+#define PPC_MSG_UNUSED 0
#define PPC_MSG_RESCHEDULE 1
#define PPC_MSG_CALL_FUNC_SINGLE 2
#define PPC_MSG_DEBUGGER_BREAK 3
diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
index 38b0ba6..bc41e9f 100644
--- a/arch/powerpc/kernel/smp.c
+++ b/arch/powerpc/kernel/smp.c
@@ -111,9 +111,9 @@ int smp_generic_kick_cpu(int nr)
}
#endif /* CONFIG_PPC64 */
-static irqreturn_t call_function_action(int irq, void *data)
+static irqreturn_t unused_action(int irq, void *data)
{
- generic_smp_call_function_interrupt();
+ /* This slot is unused and hence available for use, if needed */
return IRQ_HANDLED;
}
@@ -144,14 +144,14 @@ static irqreturn_t debug_ipi_action(int irq, void *data)
}
static irq_handler_t smp_ipi_action[] = {
- [PPC_MSG_CALL_FUNCTION] = call_function_action,
+ [PPC_MSG_UNUSED] = unused_action, /* Slot available for future use */
[PPC_MSG_RESCHEDULE] = reschedule_action,
[PPC_MSG_CALL_FUNC_SINGLE] = call_function_single_action,
[PPC_MSG_DEBUGGER_BREAK] = debug_ipi_action,
};
const char *smp_ipi_name[] = {
- [PPC_MSG_CALL_FUNCTION] = "ipi call function",
+ [PPC_MSG_UNUSED] = "ipi unused",
[PPC_MSG_RESCHEDULE] = "ipi reschedule",
[PPC_MSG_CALL_FUNC_SINGLE] = "ipi call function single",
[PPC_MSG_DEBUGGER_BREAK] = "ipi debugger",
@@ -221,8 +221,6 @@ irqreturn_t smp_ipi_demux(void)
all = xchg(&info->messages, 0);
#ifdef __BIG_ENDIAN
- if (all & (1 << (24 - 8 * PPC_MSG_CALL_FUNCTION)))
- generic_smp_call_function_interrupt();
if (all & (1 << (24 - 8 * PPC_MSG_RESCHEDULE)))
scheduler_ipi();
if (all & (1 << (24 - 8 * PPC_MSG_CALL_FUNC_SINGLE)))
@@ -265,7 +263,7 @@ void arch_send_call_function_ipi_mask(const struct cpumask
*mask)
unsigned int cpu;
for_each_cpu(cpu, mask)
- do_message_pass(cpu, PPC_MSG_CALL_FUNCTION);
+ do_message_pass(cpu, PPC_MSG_CALL_FUNC_SINGLE);
}
#if defined(CONFIG_DEBUGGER) || defined(CONFIG_KEXEC)
diff --git a/arch/powerpc/platforms/cell/interrupt.c
b/arch/powerpc/platforms/cell/interrupt.c
index 2d42f3b..28166e4 100644
--- a/arch/powerpc/platforms/cell/interrupt.c
+++ b/arch/powerpc/platforms/cell/interrupt.c
@@ -213,7 +213,7 @@ static void iic_request_ipi(int msg)
void iic_request_IPIs(void)
{
- iic_request_ipi(PPC_MSG_CALL_FUNCTION);
+ iic_request_ipi(PPC_MSG_UNUSED);
iic_request_ipi(PPC_MSG_RESCHEDULE);
iic_request_ipi(PPC_MSG_CALL_FUNC_SINGLE);
iic_request_ipi(PPC_MSG_DEBUGGER_BREAK);
diff --git a/arch/powerpc/platforms/ps3/smp.c b/arch/powerpc/platforms/ps3/smp.c
index 4b35166..488f069 100644
--- a/arch/powerpc/platforms/ps3/smp.c
+++ b/arch/powerpc/platforms/ps3/smp.c
@@ -74,7 +74,7 @@ static int __init ps3_smp_probe(void)
* to index needs to be setup.
*/
- BUILD_BUG_ON(PPC_MSG_CALL_FUNCTION!= 0);
+ BUILD_BUG_ON(PPC_MSG_UNUSED != 0);
BUILD_BUG_ON(PPC_MSG_RESCHEDULE != 1);
BUILD_BUG_ON(PPC_MSG_CALL_FUNC_SINGLE != 2);
BUILD_BUG_ON(PPC_MSG_DEBUGGER_BREAK != 3);
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[RFC V2 PATCH 3/6] cpuidle/ppc: Add timer offload framework to support deep idle states
On ppc, in deep idle states, the local clock event device of CPUs gets
switched off. On PowerPC, the local clock event device is called the
decrementer. Make use of the broadcast framework to issue interrupts to
cpus in deep idle states on their timer events, except that on ppc, we
do not have an external device such as HPET, but we use the decrementer
of one of the CPUs itself as the broadcast device.
Instantiate two different clock event devices, one representing the
decrementer and another representing the broadcast device for each cpu.
The cpu which registers its broadcast device will be responsible for
performing the function of issuing timer interrupts to CPUs in deep idle
states, and is referred to as the broadcast cpu/bc_cpu in the changelogs of this
patchset for convenience. Such a CPU is not allowed to enter deep idle
states, where the decrementer is switched off.
For now, only the boot cpu's broadcast device gets registered as a clock event
device along with the decrementer. Hence this is the broadcast cpu.
On the broadcast cpu, on each timer interrupt, apart from the regular local
timer event handler the broadcast handler is also called. We avoid the overhead
of programming the decrementer specifically for a broadcast event. The reason
is for
performance and scalability reasons. Say cpuX goes to deep idle state. It
has to ask the broadcast CPU to reprogram its(broadcast CPU's) decrementer for
the next local timer event of cpuX. cpuX can do so only by sending an IPI to the
broadcast CPU. With many more cpus going to deep idle, this model of sending
IPIs each time will result in performance bottleneck and may not scale well.
Apart from this there is no change in the way broadcast is handled today. On
a broadcast ipi the event handler for a timer interrupt is called on the cpu
in deep idle state to handle the local events.
The current design and implementation of the timer offload framework supports
the ONESHOT tick mode but not the PERIODIC mode.
Signed-off-by: Preeti U. Murthy
---
arch/powerpc/include/asm/time.h|3 +
arch/powerpc/kernel/smp.c |4 +-
arch/powerpc/kernel/time.c | 81
arch/powerpc/platforms/powernv/Kconfig |1
4 files changed, 86 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/include/asm/time.h b/arch/powerpc/include/asm/time.h
index c1f2676..936be0d 100644
--- a/arch/powerpc/include/asm/time.h
+++ b/arch/powerpc/include/asm/time.h
@@ -24,14 +24,17 @@ extern unsigned long tb_ticks_per_jiffy;
extern unsigned long tb_ticks_per_usec;
extern unsigned long tb_ticks_per_sec;
extern struct clock_event_device decrementer_clockevent;
+extern struct clock_event_device broadcast_clockevent;
struct rtc_time;
extern void to_tm(int tim, struct rtc_time * tm);
extern void GregorianDay(struct rtc_time *tm);
+extern void decrementer_timer_interrupt(void);
extern void generic_calibrate_decr(void);
extern void set_dec_cpu6(unsigned int val);
+extern int bc_cpu;
/* Some sane defaults: 125 MHz timebase, 1GHz processor */
extern unsigned long ppc_proc_freq;
diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
index 6a68ca4..d3b7014 100644
--- a/arch/powerpc/kernel/smp.c
+++ b/arch/powerpc/kernel/smp.c
@@ -114,7 +114,7 @@ int smp_generic_kick_cpu(int nr)
static irqreturn_t timer_action(int irq, void *data)
{
- timer_interrupt();
+ decrementer_timer_interrupt();
return IRQ_HANDLED;
}
@@ -223,7 +223,7 @@ irqreturn_t smp_ipi_demux(void)
#ifdef __BIG_ENDIAN
if (all & (1 << (24 - 8 * PPC_MSG_TIMER)))
- timer_interrupt();
+ decrementer_timer_interrupt();
if (all & (1 << (24 - 8 * PPC_MSG_RESCHEDULE)))
scheduler_ipi();
if (all & (1 << (24 - 8 * PPC_MSG_CALL_FUNC_SINGLE)))
diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c
index 65ab9e9..7e858e1 100644
--- a/arch/powerpc/kernel/time.c
+++ b/arch/powerpc/kernel/time.c
@@ -42,6 +42,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -97,8 +98,11 @@ static struct clocksource clocksource_timebase = {
static int decrementer_set_next_event(unsigned long evt,
struct clock_event_device *dev);
+static int broadcast_set_next_event(unsigned long evt,
+ struct clock_event_device *dev);
static void decrementer_set_mode(enum clock_event_mode mode,
struct clock_event_device *dev);
+static void decrementer_timer_broadcast(const struct cpumask *mask);
struct clock_event_device decrementer_clockevent = {
.name = "decrementer",
@@ -106,13 +110,26 @@ struct clock_event_device decrementer_clockevent = {
.irq= 0,
.set_next_event = decrementer_set_next_event,
.set_mode = decrementer_set_mod
[RFC V2 PATCH 4/6] cpuidle/ppc: Add longnap state to the idle states on powernv
This patch hooks into the existing broadcast framework along with the support
that this patchset introduces for ppc, and the cpuidle driver backend
for powernv(posted out by Deepthi Dharwar:https://lkml.org/lkml/2013/7/23/128)
to add sleep state as one of the deep idle states, in which the decrementer
is switched off.
However in this patch, we only emulate sleep by going into a state which does
a nap with the decrementer interrupts disabled, termed as longnap. This enables
focus on the timer broadcast framework for ppc in this series of patches ,
which is required as a first step to enable sleep on ppc.
Signed-off-by: Preeti U Murthy
---
arch/powerpc/platforms/powernv/processor_idle.c | 48 +++
1 file changed, 47 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/platforms/powernv/processor_idle.c
b/arch/powerpc/platforms/powernv/processor_idle.c
index f43ad91a..9aca502 100644
--- a/arch/powerpc/platforms/powernv/processor_idle.c
+++ b/arch/powerpc/platforms/powernv/processor_idle.c
@@ -9,16 +9,18 @@
#include
#include
#include
+#include
#include
#include
+#include
struct cpuidle_driver powernv_idle_driver = {
.name = "powernv_idle",
.owner =THIS_MODULE,
};
-#define MAX_IDLE_STATE_COUNT 2
+#define MAX_IDLE_STATE_COUNT 3
static int max_idle_state = MAX_IDLE_STATE_COUNT - 1;
static struct cpuidle_device __percpu *powernv_cpuidle_devices;
@@ -54,6 +56,43 @@ static int nap_loop(struct cpuidle_device *dev,
return index;
}
+/* Emulate sleep, with long nap.
+ * During sleep, the core does not receive decrementer interrupts.
+ * Emulate sleep using long nap with decrementers interrupts disabled.
+ * This is an initial prototype to test the timer offload framework for ppc.
+ * We will eventually introduce the sleep state once the timer offload
framework
+ * for ppc is stable.
+ */
+static int longnap_loop(struct cpuidle_device *dev,
+ struct cpuidle_driver *drv,
+ int index)
+{
+ int cpu = dev->cpu;
+
+ unsigned long lpcr = mfspr(SPRN_LPCR);
+
+ lpcr &= ~(LPCR_MER | LPCR_PECE); /* lpcr[mer] must be 0 */
+
+ /* exit powersave upon external interrupt, but not decrementer
+* interrupt, Emulate sleep.
+*/
+ lpcr |= LPCR_PECE0;
+
+ if (cpu != bc_cpu) {
+ mtspr(SPRN_LPCR, lpcr);
+ clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ENTER, &cpu);
+ power7_nap();
+ clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_EXIT, &cpu);
+ } else {
+ /* Wakeup on a decrementer interrupt, Do a nap */
+ lpcr |= LPCR_PECE1;
+ mtspr(SPRN_LPCR, lpcr);
+ power7_nap();
+ }
+
+ return index;
+}
+
/*
* States for dedicated partition case.
*/
@@ -72,6 +111,13 @@ static struct cpuidle_state
powernv_states[MAX_IDLE_STATE_COUNT] = {
.exit_latency = 10,
.target_residency = 100,
.enter = &nap_loop },
+{ /* LongNap */
+ .name = "LongNap",
+ .desc = "LongNap",
+ .flags = CPUIDLE_FLAG_TIME_VALID,
+ .exit_latency = 10,
+ .target_residency = 100,
+ .enter = &longnap_loop },
};
static int powernv_cpuidle_add_cpu_notifier(struct notifier_block *n,
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[RFC V2 PATCH 5/6] cpuidle/ppc: Enable dynamic movement of the broadcast functionality across CPUs
In the current design of the timer offload framework for powerpc, there is a
dedicated broadcast CPU, which is the boot CPU. But this is not good because:
a.It disallows this CPU from being hotplugged out.
b.Overburdening this CPU with the broadcast duty can take
a toll on the performance, which could worsen if this CPU
is already too busy.
c.This could lead to thermal or power imbalance within the chip.
To overcome the above constraints, float around the duty of doing a broadcast
around the CPUs. The current design proposes to choose the first CPU that
attempts to go to deep idle state to be the broadcast CPU/bc_cpu.
It is disallowed from entering deep idle state.
Let the broadcast CPU become invalidated when there are no more CPUs in
the broadcast mask. Until this point the rest of the CPUs attempting to enter
deep idle will be allowed to do so, to be woken up by the broadcast CPU.
Hence the set and unset of the bc_cpu variable is done only by the broadcast
CPU.
Protect the region of all the above activity with a lock in order to avoid
race conditions between readers and writers of the bc_cpu
entry and the broadcast cpus mask. One typical scenario could be:
CPUACPUB
Read bc_cpu exists Is the bc_cpu, finds the broadcast mask
empty,and invalidates the bc_cpu.
Enter itself into the
the broadcast mask.
Thus, CPUA would go into deep idle when broadcast handling is inactive.
The broadcast clockevent device is now one single pseudo device capable of
working for
all possible cpus (instead of being per-cpu like it was before, there is no
point in having so), due to the dynamic movement of the broadcast CPU. This
is a pseudo device and the dynamic movement of bc_cpu will therefore not affect
its functioning. The broadcast clockevent device's event handler will be called
by the bc_cpu in each of its timer interrupt.
This patchset adds hotplug notifiers to change the bc_cpu, in case it goes
offline. In this case choose the first cpu in the broadcast mask to be the
next bc_cpu and send an IPI, to wake it up to begin to handle broadcast events
thereon. This IPI is the same as the broadcast IPI.
The idea being the intention of both these scenarios(hotplug and actual
broadcast wakeup) is to wake up a CPU in the broadcast mask, except that
they are for different reasons.
Signed-off-by: Preeti U Murthy
---
arch/powerpc/include/asm/time.h |1
arch/powerpc/kernel/time.c | 10 ++--
arch/powerpc/platforms/powernv/processor_idle.c | 56 +++
3 files changed, 53 insertions(+), 14 deletions(-)
diff --git a/arch/powerpc/include/asm/time.h b/arch/powerpc/include/asm/time.h
index 936be0d..92260c9 100644
--- a/arch/powerpc/include/asm/time.h
+++ b/arch/powerpc/include/asm/time.h
@@ -25,6 +25,7 @@ extern unsigned long tb_ticks_per_usec;
extern unsigned long tb_ticks_per_sec;
extern struct clock_event_device decrementer_clockevent;
extern struct clock_event_device broadcast_clockevent;
+extern struct clock_event_device bc_timer;
struct rtc_time;
extern void to_tm(int tim, struct rtc_time * tm);
diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c
index 7e858e1..a19c8ca 100644
--- a/arch/powerpc/kernel/time.c
+++ b/arch/powerpc/kernel/time.c
@@ -127,9 +127,9 @@ EXPORT_SYMBOL(broadcast_clockevent);
DEFINE_PER_CPU(u64, decrementers_next_tb);
static DEFINE_PER_CPU(struct clock_event_device, decrementers);
-static DEFINE_PER_CPU(struct clock_event_device, bc_timer);
+struct clock_event_device bc_timer;
-int bc_cpu;
+int bc_cpu = -1;
#define XSEC_PER_SEC (1024*1024)
#ifdef CONFIG_PPC64
@@ -504,7 +504,7 @@ void timer_interrupt(struct pt_regs * regs)
struct pt_regs *old_regs;
u64 *next_tb = &__get_cpu_var(decrementers_next_tb);
struct clock_event_device *evt = &__get_cpu_var(decrementers);
- struct clock_event_device *bc_evt = &__get_cpu_var(bc_timer);
+ struct clock_event_device *bc_evt = &bc_timer;
int cpu = smp_processor_id();
u64 now;
@@ -879,10 +879,10 @@ static void register_decrementer_clockevent(int cpu)
static void register_broadcast_clockevent(int cpu)
{
- struct clock_event_device *bc_evt = &per_cpu(bc_timer, cpu);
+ struct clock_event_device *bc_evt = &bc_timer;
*bc_evt = broadcast_clockevent;
- bc_evt->cpumask = cpumask_of(cpu);
+ bc_evt->cpumask = cpu_possible_mask;
printk_once(KERN_DEBUG "clockevent: %s mult[%x] shift[%d] cpu[%d]\n",
bc_evt->name, bc_evt->mult, bc_evt->shift, cpu);
diff --git a/arch/powerpc/platforms/powernv/processor_idle.c
b/arch/powerpc/platforms/powernv/processor_idle.c
index 9aca502..9554da6 100644
--- a/arch/powerpc/platforms/powernv/processor_idle.c
+++ b/arch/powerpc/platforms/powernv/processor_idle.c
@@ -10,6 +10,8 @@
#include
#include
#include
+#include
+#include
#include
#inclu
[RFC V2 PATCH 6/6] cpuidle/ppc : Queue a hrtimer on bc_cpu, explicitly to do broadcast handling
In the current design we were depending on the timer interrupt on the
bc_cpu to trigger broadcast handling. In tickless idle, timer interrupts
could be many ticks away which could result in missed wakeups on CPUs in deep
idle states. Disabling tickless idle on the bc_cpu is not good for
powersavings.
Therefore queue a hrtimer specifically for broadcast handling. When the
broadcast
CPU is chosen, it schedules this hrtimer to fire after a jiffy.
This is meant to initiate broadcast handling. For each expiration of
this hrtimer thereon, it is reprogrammed to fire at the time the next broadcast
handling has to be done. But if there is no pending broadcast handling to be
done in the future, the broadcast cpu is invalidated and the hrtimer is
cancelled. The above cycle repeats when the next CPU attempts to enter sleep
state.
Of course the time at which the hrtimer fires initially can be scheduled for
time=target_residency of deep idle state instead of a jiffy, since CPUs going
into deep idle states will not have their next wakeup event before this
target_residency time of the the deep idle state.
But this patchset is based on longnap which is now used to mimick sleep
but is actually nap with decrementer interrupts disabled. Therefore its
target_residency is the same as nap. The CPUs going into longnap, will
probably need to be woken up sooner than they would have been,had they gone into
sleep. Hence the initial scheduling of the hrtimer is held at a jiffy as of now.
There is one other significant point. On CPU hotplug, the hrtimer on the
broadcast CPU is cancelled, the bc_cpu entry is invalidated, a new
broadcast cpu is chosen as before, and an IPI is sent to it. However instead
of using a broadcast IPI to wake it up, use smp_call_function_single(),
because apart from just wakeup, the new broadcast CPU has to restart
the hrtimer on itself so as to continue broadcast handling.
Signed-off-by: Preeti U Murthy
---
arch/powerpc/include/asm/time.h |5 ++
arch/powerpc/kernel/time.c | 47 ---
arch/powerpc/platforms/powernv/processor_idle.c | 38 ++-
3 files changed, 73 insertions(+), 17 deletions(-)
diff --git a/arch/powerpc/include/asm/time.h b/arch/powerpc/include/asm/time.h
index 92260c9..b9a60eb 100644
--- a/arch/powerpc/include/asm/time.h
+++ b/arch/powerpc/include/asm/time.h
@@ -16,6 +16,7 @@
#ifdef __KERNEL__
#include
#include
+#include
#include
@@ -26,6 +27,7 @@ extern unsigned long tb_ticks_per_sec;
extern struct clock_event_device decrementer_clockevent;
extern struct clock_event_device broadcast_clockevent;
extern struct clock_event_device bc_timer;
+extern struct hrtimer *bc_hrtimer;
struct rtc_time;
extern void to_tm(int tim, struct rtc_time * tm);
@@ -35,7 +37,10 @@ extern void decrementer_timer_interrupt(void);
extern void generic_calibrate_decr(void);
extern void set_dec_cpu6(unsigned int val);
+extern ktime_t get_next_bc_tick(void);
+extern enum hrtimer_restart handle_broadcast(struct hrtimer *hrtimer);
extern int bc_cpu;
+extern int bc_hrtimer_initialized;
/* Some sane defaults: 125 MHz timebase, 1GHz processor */
extern unsigned long ppc_proc_freq;
diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c
index a19c8ca..1a64d58 100644
--- a/arch/powerpc/kernel/time.c
+++ b/arch/powerpc/kernel/time.c
@@ -43,6 +43,8 @@
#include
#include
#include
+#include
+#include
#include
#include
#include
@@ -128,6 +130,8 @@ EXPORT_SYMBOL(broadcast_clockevent);
DEFINE_PER_CPU(u64, decrementers_next_tb);
static DEFINE_PER_CPU(struct clock_event_device, decrementers);
struct clock_event_device bc_timer;
+struct hrtimer *bc_hrtimer;
+int bc_hrtimer_initialized = 0;
int bc_cpu = -1;
#define XSEC_PER_SEC (1024*1024)
@@ -504,8 +508,6 @@ void timer_interrupt(struct pt_regs * regs)
struct pt_regs *old_regs;
u64 *next_tb = &__get_cpu_var(decrementers_next_tb);
struct clock_event_device *evt = &__get_cpu_var(decrementers);
- struct clock_event_device *bc_evt = &bc_timer;
- int cpu = smp_processor_id();
u64 now;
/* Ensure a positive value is written to the decrementer, or else
@@ -551,10 +553,6 @@ void timer_interrupt(struct pt_regs * regs)
*next_tb = ~(u64)0;
if (evt->event_handler)
evt->event_handler(evt);
- if (cpu == bc_cpu && bc_evt->event_handler) {
- bc_evt->event_handler(bc_evt);
- }
-
} else {
now = *next_tb - now;
if (now <= DECREMENTER_MAX)
@@ -864,6 +862,42 @@ static void decrementer_timer_broadcast(const struct
cpumask *mask)
arch_send_tick_broadcast(mask);
}
+ktime_t get_next_bc_tick(void)
+{
+ u64 next_bc_ns;
+
+ next_bc_ns = (tb_ticks_per_jiffy / tb_ticks_per_usec) * 1000;
+ return ns_to_ktime(next_bc_ns);
+}
+
+enum hrtimer_restart handl
Re: [PATCH v4 resent 1/2] ASoC: fsl: Add S/PDIF CPU DAI driver
Am Mittwoch, den 14.08.2013, 16:48 +0800 schrieb Nicolin Chen:
> Hi Sascha,
>
> On Wed, Aug 14, 2013 at 09:50:17AM +0200, Sascha Hauer wrote:
> > > + - tx-clksrc-names : The names for all available clock sources for tx,
> > > which
> > > + is also being listed in SoC reference manual, ClkSrc_Sel bit of
> > > SPDIF_SRPC.
> > > + And the name list would be different between different SoC. Use 'null'
> > > for
> > > + those unlisted names, and the max number of tx-clksrc-names should be
> > > 8.
> > > +
> > > + - rx-clksrc-names : The names for all available clock sources for rx,
> > > which
> > > + is also being listed in SoC reference manual, TxClk_Source bit of
> > > SPDIF_STC.
> > > + And the name list would be different between different SoC. Use 'null'
> > > for
> > > + those unlisted names, and the max number of rx-clksrc-names should be
> > > 16.
> > > +
> > > +Optional properties:
> > > +
> > > + - rx-clksrc-lock: This is a boolean property. If present, ClkSrc_Sel
> > > bit
> > > + of SPDIF_SRPC would be set a clock source that cares DPLL locked
> > > condition.
> > > +
> > > +Example1:
> > > +
> > > +spdif: spdif@02004000 {
> > > + clocks = <&clks 197>;
> > > + clock-names = "core";
> > > + rx-clksrc-lock;
> > > + rx-clksrc-names =
> > > + "lock.ext", "lock.spdif", "lock.asrc",
> > > + "lock.spdif_ext", "lock.esai", "ext",
> > > + "spdif", "asrc", "spdif_ext", "esai",
> > > + "lock.mlb", "lock.mlb_phy", "mlb",
> > > + "mlb_phy";
> > > + tx-clksrc-names =
> > > + "xtal", "spdif", "asrc", "spdif_ext",
> > > + "esai", "ipg", "mlb", "mlb_phy";
> >
> > I had a hard time understanding what you are doing here.
> >
> > With this the clk names in arch/arm/mach-imx/clk-imx6q.c become an API
> > between the Kernel and the devicetree. Don't do that.
> >
> > There is a standardized devicetree binding for clocks. Use it.
>
> I think I should first explain to you what this part is doing:
> The driver needs to set Clk_source bit for TX/RX to select the
> clock from a clock mux. The names listed above are those of the
> clocks connecting to the mux, while they are not only internal
> clocks which're included in clk-imx6q.c but also external ones,
> an on-board external osc for example.
>
> The driver does get the clock by using the standard DT binding,
> see the 'clocks = <&clks 197>' above, and then compare this
> obtained clock->name with the name list to decide which value
> should be set to the Clk_source bit.
>
> ==
> ClkSrc_Sel from i.MX6Q reference manual:
>
> Clock source selection, all other settings not shown are reserved:
> if (DPLL Locked) SPDIF_RxClk else extal
> 0001 if (DPLL Locked) SPDIF_RxClk else spdif_clk
> 0010 if (DPLL Locked) SPDIF_RxClk else asrc_clk
> 0011 if (DPLL Locked) SPDIF_RxClk else spdif_extclk
> 0100 if (DPLL Locked) SPDIF_Rxclk else esai_hckt
> 0101 extal_clk
> 0110 spdif_clk
> 0111 asrc_clk
> 1000 spdif_extclk
> 1001 esai_hckt
> 1010 if (DPLL Locked) SPDIF_RxClk else mlb_clk
> ==
==
>From i.MX53 reference manual:
if (DPLL Locked) SPDIF_RxClk else extal
0001 if (DPLL Locked) SPDIF_RxClk else spdif_clk
0011 if (DPLL Locked) SPDIF_RxClk else asrc_clk
0100 if (DPLL Locked) SPDIF_Rxclk else esai_hckt
0101 extal_clk
0110 spdif_clk
1000 asrc_clk
1001 esai_hckt
1010 if (DPLL Locked) SPDIF_RxClk else mlb_clk
1011 if (DPLL Locked) SPDIF_RxClk else camp_clk
1100 mkb_clk
1101 camp_clk
==
To me this looks like the device tree should just contain the list of
unique clock inputs using phandles.
/* for i.MX6Q: */
clocks = <&...>;
clock-names = "xtal", "spdif", "asrc",
"spdif_ext", "esai", "mlb";
/* for i.MX53: */
clocks = <&...>;
clock-names = "xtal", "spdif", "asrc",
"esai", "mlb", "camp";
The driver could contain this list of named inputs to the multiplexer
and the DPLL locking information for each SoC version. The per-clock
DPLL locking bit shouldn't be in the device tree at all.
> So the name list here basically is not being used to obtain a
> clock like what standardized DT binding does but to provide the
> driver a full list to look up which value should be exactly used
> according to the obtained clock.
>
> I think I should revise the binding doc for these two lists. It
> might be hard to explain within that kinda short paragraph.
>
> Surely, if I misunderstand your point, please correct me. And
> if you have any sage idea, please guide me.
regards
Philipp
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [alsa-devel] [PATCH v4 resent 1/2] ASoC: fsl: Add S/PDIF CPU DAI driver
On Wed, Aug 14, 2013 at 06:23:46PM +0800, Nicolin Chen wrote: > Hi, > > > > Surely, if I misunderstand your point, please correct me. And > > > if you have any sage idea, please guide me. > > > > Something like this: > > > > clocks = <&clks 197>, <&clks 3>, <&clks 197>, <&clks 107>, <&clks > > SPDIF_EXT>, > > <&clks 118>, <&clks 62>, <&clks 139>, <&clks MLB_PHY> > > clock-names = "core", "rxtx0", "rxtx1", "rxtx2", "rxtx3", "rxtx4", "rxtx5", > > "rxtx6", "rxtx7" > > > > This describes the different input clocks to the spdif core and also > > gives a hint to the array index (rxtx_n_) to use. > > Thank you for the idea, and..hmm..I'm a bit confused.. Is this really > a nicer way? Yes, since the clk names are not an API. Exposing them to the devicetree is not an option. The fact that the names are defined in arch/arm/mach-imx/clk-imx6q.c and are used in the spdif driver makes this really clear. > > Actually the rx clock list and tx clock list are totally different. > So doing this I have to list, in the maximum case, 24 (8 + 16) clock > phandles for these two lists. And plussing another 6 I've listed in > this binding doc -- thus there are totally 30 clock phanldes. But > the 24 of 30 are only used to get two indexes. The spdif core has 8 input clocks which have to be described in the devicetree. Nobody says the mapping which clock name corresponds to which bit combination has to be in the devicetree. Look at the possible clocks: if (DPLL Locked) SPDIF_RxClk else extal 0001 if (DPLL Locked) SPDIF_RxClk else spdif_clk 0010 if (DPLL Locked) SPDIF_RxClk else asrc_clk 0011 if (DPLL Locked) SPDIF_RxClk else spdif_extclk 0100 if (DPLL Locked) SPDIF_Rxclk else esai_hckt 0101 extal_clk 0110 spdif_clk 0111 asrc_clk 1000 spdif_extclk 1001 esai_hckt 1010 if (DPLL Locked) SPDIF_RxClk else mlb_clk 1011 if (DPLL Locked) SPDIF_RxClk else mlb_phy_clk 1100 mkb_clk 1101 mlb_phy_clk Only half of them actually are clocks. "if (DPLL Locked) SPDIF_RxClk else ..." is not a clock. Every sane hardware developer would have introduced a mux with 8 entries and an additional "Use DPLL if possible" bit. Now this is not the case here so we have to live with it and maintain the above table in the driver. And another one for the i.MX35 and still another one for i.MX53. > > I think I need a little help here to understand why this is better. As said, the clock names are not an API. Nobody changing clk-imx6q.c expects to break devicetree bindings. Sascha -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [GIT PULL] DT/core: cpu_ofnode updates for v3.12
On 08/14/2013 05:01 AM, Sudeep KarkadaNagesha wrote:
> On 13/08/13 22:07, Benjamin Herrenschmidt wrote:
>> On Tue, 2013-08-13 at 19:29 +0100, Sudeep KarkadaNagesha wrote:
>>> I don't understand completely the use of ibm,ppc-interrupt-server#s and
>>> its implications on generic of_get_cpu_node implementation.
>>> I see the PPC specific definition of of_get_cpu_node uses thread id only
>>> in 2 instances. Based on that, I have tried to move all the other
>>> instances to use generic definition.
>>>
>>> Let me know if the idea is correct.
>>
>> No. The device-tree historically only represents cores, not HW threads, so
>> it makes sense to retrieve also the thread number corresponding to the CPU.
>>
> Ok
>
>> However, the mechanism to represent HW threads in the device-tree is
>> currently
>> somewhat platform specific (the ibm,ppc-interrupt-server#s).
> I see most of the callers pass NULL to thread id argument except 2
> instances in entire tree. If that's the case why can't we move to use
> generic of_get_cpu_node in most of those cases and have PPC specific
> implementation for the ones using thread id.
>
>>
>> So what you could do for now is:
>>
>> - Have a generic version that always returns 0 as the thread, which is weak
> I would prefer to move to generic of_get_cpu_node where ever possible
> and rename the function that takes thread id rather than making generic
> one weak.
>
>>
>> - powerpc keeps its own implementation
> How about only in cases where it needs thread_id.
>
>>
>> - Start a discussion on the bindings (if not already there) to define
>> threads
>> in a better way at which point the generic function can be updated.
>>
> I am not sure if we need to define any new bindings. Excerpts from ePAPR
> (v1.1):
> "3.7.1 General Properties of CPU nodes
> The value of "reg" is a that defines a unique
> CPU/thread id for the CPU/threads represented by the CPU node.
> If a CPU supports more than one thread (i.e. multiple streams of
> execution) the reg property is an array with 1 element per thread. The
> #address-cells on the /cpus node specifies how many cells each element
> of the array takes. Software can determine the number of threads by
> dividing the size of reg by the parent node's #address-cells."
>
> And this is exactly in agreement to what's implemented in the generic
> of_get_cpu_node:
>
> for_each_child_of_node(cpus, cpun) {
> if (of_node_cmp(cpun->type, "cpu"))
> continue;
> cell = of_get_property(cpun, "reg", &prop_len);
> if (!cell) {
> pr_warn("%s: missing reg property\n", cpun->full_name);
> continue;
> }
> prop_len /= sizeof(*cell);
> while (prop_len) {
> hwid = of_read_number(cell, ac);
> prop_len -= ac;
> if (arch_match_cpu_phys_id(cpu, hwid))
> return cpun;
> }
> }
How about something like this:
for_each_child_of_node(cpus, cpun) {
if (of_node_cmp(cpun->type, "cpu"))
continue;
if (arch_of_get_cpu_node(cpun, thread))
return cpun;
cell = of_get_property(cpun, "reg", &prop_len);
if (!cell) {
pr_warn("%s: missing reg property\n", cpun->full_name);
continue;
}
prop_len /= sizeof(*cell);
while (prop_len) {
hwid = of_read_number(cell, ac);
prop_len -= ac;
if (arch_match_cpu_phys_id(cpu, hwid))
return cpun;
}
}
For PPC:
arch_of_get_cpu_node()
{
const u32 *intserv;
unsigned int plen, t;
/* Check for ibm,ppc-interrupt-server#s. */
intserv = of_get_property(np, "ibm,ppc-interrupt-server#s",
&plen);
if (!intserv)
return false;
hardid = get_hard_smp_processor_id(cpu);
plen /= sizeof(u32);
for (t = 0; t < plen; t++) {
if (hardid == intserv[t]) {
if (thread)
*thread = t;
return true;
}
}
return false;
}
>
> Yes this doesn't cover the historical "ibm,ppc-interrupt-server#s", for
> which we can have PPC specific wrapper above the generic one i.e. get
> the cpu node and then parse for thread id under custom property.
>
> Let me know your thoughts.
>
> Regards,
> Sudeep
>
>
>
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [GIT PULL] DT/core: cpu_ofnode updates for v3.12
On 14/08/13 12:37, Benjamin Herrenschmidt wrote: > On Wed, 2013-08-14 at 11:01 +0100, Sudeep KarkadaNagesha wrote: >> Yes this doesn't cover the historical "ibm,ppc-interrupt-server#s", >> for >> which we can have PPC specific wrapper above the generic one i.e. get >> the cpu node and then parse for thread id under custom property. > > A wrapper is wrong. I don't want to have to have all ppc callers to use > a different function. Ok. On the side note the main intention of this patch series[1] is to avoid calling of_get_cpu_node function once CPU devices are registered. It initialises the of_node in all the cpu devices using this function when registering the CPU device. It can be retrieved from cpu_dev->of_node. So direct users of of_get_cpu_node can be reduced to avoid unnecessary parsing of DT to find cpu node. > > As I said, just make a generic one that returns a thread ID, ie, same > signature as the powerpc one. Make it weak, we can override it in > powerpc-land, IMO, making generic definition which adhere to the ePAPR specification as weak is not so clean. > or we can move the ibm,ppc-interrupt-server#s handling > into the generic one, it won't hurt, but leave the thread_id return > there, it doesn't hurt it will come in handy in a few cases without > causing code duplication. > IMO moving of handling ibm,ppc-interrupt-server#s to generic code under #ifdef CONFIG_PPC seems to be cleaner approach than weak definitation. As per my understanding each thread is a different logical cpu. Each logical cpu is mapped to unique physical id(either present in reg field or legacy ibm,ppc-interrupt-server#s field). So given a logical cpu id we can get the cpu node corresponding to it. Looking @ smp_setup_cpu_maps in arch/powerpc/kernel/setup-common.c and the comment in the same file: "This implementation only supports power of 2 number of threads.." the thread id id is implicit in the logical cpu id. Do we need to fetch that from DT ? Regards, Sudeep [1] https://lkml.org/lkml/2013/7/22/219 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [GIT PULL] DT/core: cpu_ofnode updates for v3.12
On 14/08/13 13:53, Rob Herring wrote:
> On 08/14/2013 05:01 AM, Sudeep KarkadaNagesha wrote:
>> On 13/08/13 22:07, Benjamin Herrenschmidt wrote:
>>> On Tue, 2013-08-13 at 19:29 +0100, Sudeep KarkadaNagesha wrote:
I don't understand completely the use of ibm,ppc-interrupt-server#s and
its implications on generic of_get_cpu_node implementation.
I see the PPC specific definition of of_get_cpu_node uses thread id only
in 2 instances. Based on that, I have tried to move all the other
instances to use generic definition.
Let me know if the idea is correct.
>>>
>>> No. The device-tree historically only represents cores, not HW threads, so
>>> it makes sense to retrieve also the thread number corresponding to the CPU.
>>>
>> Ok
>>
>>> However, the mechanism to represent HW threads in the device-tree is
>>> currently
>>> somewhat platform specific (the ibm,ppc-interrupt-server#s).
>> I see most of the callers pass NULL to thread id argument except 2
>> instances in entire tree. If that's the case why can't we move to use
>> generic of_get_cpu_node in most of those cases and have PPC specific
>> implementation for the ones using thread id.
>>
>>>
>>> So what you could do for now is:
>>>
>>> - Have a generic version that always returns 0 as the thread, which is weak
>> I would prefer to move to generic of_get_cpu_node where ever possible
>> and rename the function that takes thread id rather than making generic
>> one weak.
>>
>>>
>>> - powerpc keeps its own implementation
>> How about only in cases where it needs thread_id.
>>
>>>
>>> - Start a discussion on the bindings (if not already there) to define
>>> threads
>>> in a better way at which point the generic function can be updated.
>>>
>> I am not sure if we need to define any new bindings. Excerpts from ePAPR
>> (v1.1):
>> "3.7.1 General Properties of CPU nodes
>> The value of "reg" is a that defines a unique
>> CPU/thread id for the CPU/threads represented by the CPU node.
>> If a CPU supports more than one thread (i.e. multiple streams of
>> execution) the reg property is an array with 1 element per thread. The
>> #address-cells on the /cpus node specifies how many cells each element
>> of the array takes. Software can determine the number of threads by
>> dividing the size of reg by the parent node's #address-cells."
>>
>> And this is exactly in agreement to what's implemented in the generic
>> of_get_cpu_node:
>>
>> for_each_child_of_node(cpus, cpun) {
>> if (of_node_cmp(cpun->type, "cpu"))
>> continue;
>> cell = of_get_property(cpun, "reg", &prop_len);
>> if (!cell) {
>> pr_warn("%s: missing reg property\n", cpun->full_name);
>> continue;
>> }
>> prop_len /= sizeof(*cell);
>> while (prop_len) {
>> hwid = of_read_number(cell, ac);
>> prop_len -= ac;
>> if (arch_match_cpu_phys_id(cpu, hwid))
>> return cpun;
>> }
>> }
>
> How about something like this:
>
> for_each_child_of_node(cpus, cpun) {
> if (of_node_cmp(cpun->type, "cpu"))
> continue;
>
> if (arch_of_get_cpu_node(cpun, thread))
> return cpun;
>
> cell = of_get_property(cpun, "reg", &prop_len);
> if (!cell) {
> pr_warn("%s: missing reg property\n", cpun->full_name);
> continue;
> }
> prop_len /= sizeof(*cell);
> while (prop_len) {
> hwid = of_read_number(cell, ac);
> prop_len -= ac;
> if (arch_match_cpu_phys_id(cpu, hwid))
> return cpun;
> }
> }
>
> For PPC:
>
> arch_of_get_cpu_node()
> {
> const u32 *intserv;
> unsigned int plen, t;
>
> /* Check for ibm,ppc-interrupt-server#s. */
> intserv = of_get_property(np, "ibm,ppc-interrupt-server#s",
> &plen);
> if (!intserv)
> return false;
>
> hardid = get_hard_smp_processor_id(cpu);
>
> plen /= sizeof(u32);
> for (t = 0; t < plen; t++) {
> if (hardid == intserv[t]) {
> if (thread)
> *thread = t;
> return true;
> }
> }
> return false;
> }
>
Sorry responded to earlier mail before seeing this. This approach looks
good, but we still need to have thread id as argument which should be fine.
But as per my understanding on how logical cpu<->hard proccessor id is
setup, the thread_id is implicit in the logical cpu id making it
unnecessary to depend on DT each time.
Regards,
Sudeep
>>
>> Yes this doesn't cover the historical "ibm,ppc-interrupt-server#s", for
>> which we can have PPC specific wrapper above the generic one i.e. get
>> the cpu node and then parse for thread id under custom property.
>>
>> Let me
Re: [alsa-devel] [PATCH v4 1/2] ASoC: fsl: Add S/PDIF CPU DAI driver
On 08/14/2013 02:14 AM, Shawn Guo wrote: > On Wed, Aug 14, 2013 at 02:34:45PM +0800, Nicolin Chen wrote: >> On Wed, Aug 14, 2013 at 02:39:33PM +0800, Shawn Guo wrote: >>> We only need to maintain those versions that require different >>> programming model in the list. For example, if S/PDIF on Vybrid >>> is completely compatible with imx6q one and uses the exactly same >>> programming model, we do not need to maintain a compatible string >>> for Vybrid S/PDIF at all. Instead, we only need to have something >>> like below in Vybrid dts file, and S/PDIF driver will just work for it. >>> >>> compatible = "fsl,vf600-spdif", "fsl,imx6q-spdif"; >>> >>> Shawn >> >> Clear. Thank you for the explain. >> >> Then I think I can merely remain "fsl,imx6q-spdif" here, >> because all other cases should be completely compatible >> with this one. They are only different in the clock source >> names list, which's already being specified in dts file. >> >> Please correct me if you think this still isn't proper. If the clock source name list is different, then it needs a different compatible value, so that each compatible value can specify which clock names are required. Also, the compatible value itself should always include the exact HW that's present (most specific HW version), as well as any other HW it's compatible with. > And we generally prefer to use the soc that firstly integrates the IP to > name the compatible. For IMX series, I think imx35 is the one, so we > would name it "fsl,imx35-spdif". ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Embedded System Question
Hi, I have to struggle with a broadcast video decoder with an unknown PPC Linux. I would like to compile a new module, but I need a build environment. Here some infos about the system: Linux version 2.6.14-gemppc-iolite processor : 0 cpu : 405GPr clock : 399MHz revision: 9.81 (pvr 5091 0951) bogomips: 398.33 machine : GemPPC Iolite plb bus clock : 133MHz pci bus clock : 33MHz GNU/Linux 2.6.14 port for the GemPPC Iolite platform Is there a chance to work with this semi closed system. Thanks in advance for all input. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 1/3] powerpc: Add iommu domain pointer to device archdata
On Wed, Aug 14, 2013 at 09:56:11AM +, Sethi Varun-B16395 wrote: > Please find the .config file attached with this mail. Fantastic, thanks. The build works fine, I'll include the driver into my next-branch. I also have two minor clean-up patches on-top, just if you where wondering. Joerg ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 1/3] powerpc: Add iommu domain pointer to device archdata
Thanks Joerg. > -Original Message- > From: Joerg Roedel [mailto:j...@8bytes.org] > Sent: Wednesday, August 14, 2013 9:45 PM > To: Sethi Varun-B16395 > Cc: io...@lists.linux-foundation.org; linuxppc-dev@lists.ozlabs.org; > linux-ker...@vger.kernel.org; b...@kernel.crashing.org; > ga...@kernel.crashing.org; alex.william...@redhat.com; Yoder Stuart- > B08248; Wood Scott-B07421 > Subject: Re: [PATCH 1/3] powerpc: Add iommu domain pointer to device > archdata > > On Wed, Aug 14, 2013 at 09:56:11AM +, Sethi Varun-B16395 wrote: > > Please find the .config file attached with this mail. > > Fantastic, thanks. The build works fine, I'll include the driver into my > next-branch. I also have two minor clean-up patches on-top, just if you > where wondering. > > > Joerg > > ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: Pull request: scottwood/linux.git next
On Wed, 2013-08-14 at 14:18 +1000, Benjamin Herrenschmidt wrote: > On Thu, 2013-08-08 at 17:45 -0500, Scott Wood wrote: > > powerpc/e500: Update compilation flags with core specific > > options > > This breaks the build for my FSL test configs. For some reason gcc 4.7.3 > doesn't know about -mcpu=e5500 Ugh. I guess that's what I get for using toolchains provided internally rather than building them myself -- though it doesn't help that the GCC people love finding new ways to break building GCC without libc (why doesn't --without-headers automatically disable any component that requires libc headers?), and usually respond to bug reports with "go run crosstool and leave us alone". It looks like e5500 support is in 4.8.1, but I can't actually get it to build (libdecnumber wants to #include_next and can't be disabled -- arm64 toolchain built fine with the similar configure options). I don't know about earlier versions. > Additionally, on 64-bit, that means one can no longer make a kernel that > does both A2 and e5500... Other than the toolchain issue, I'm not sure how this is worse than it was before, when such a kernel would have had -Wa,-me500 forced. What -mcpu value should be used in such a combined kernel? > I'm reverting that crap patch, please make such optimizations CONFIG_* > options like power5...7 Speaking of crap patches, those config options don't limit themselves to book3s and thus we're now getting CONFIG_GENERIC_CPU (and thus -mtune=power7) on e5500 builds. -Scott ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH RESEND 0/8] use platform_{get,set}_drvdata()
On 08/14/2013 07:36 AM, Libo Chen wrote:
We can use the wrapper functions platform_{get,set}_drvdata() instead of
dev_{get,set}_drvdata() with &pdev->dev, it is convenient for user.
Also, unnecessary dev_set_drvdata() is removed, because the driver core
clears the driver data to NULL after device_release or on probe failure.
Saying "also" in the changelog is usually a good sign that the patch
should be split.
WBR, Sergei
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: Pull request: scottwood/linux.git next
On Wed, 2013-08-14 at 12:02 -0500, Scott Wood wrote: > On Wed, 2013-08-14 at 14:18 +1000, Benjamin Herrenschmidt wrote: > > On Thu, 2013-08-08 at 17:45 -0500, Scott Wood wrote: > > > powerpc/e500: Update compilation flags with core specific > > > options > > > > This breaks the build for my FSL test configs. For some reason gcc 4.7.3 > > doesn't know about -mcpu=e5500 > > Ugh. I guess that's what I get for using toolchains provided internally > rather than building them myself :-) I recommend you use one of tony's on kernel.org, that way you have something that matches what other people use :-) > -- though it doesn't help that the GCC > people love finding new ways to break building GCC without libc (why > doesn't --without-headers automatically disable any component that > requires libc headers?) Hehe, yeah. > , and usually respond to bug reports with "go run > crosstool and leave us alone". It looks like e5500 support is in 4.8.1, > but I can't actually get it to build (libdecnumber wants to > #include_next and can't be disabled -- arm64 toolchain built > fine with the similar configure options). I don't know about earlier > versions. In any case, there are mechanisms in Kbuild to check if the option exist, so that should be used here. Look at the other ones. > > Additionally, on 64-bit, that means one can no longer make a kernel that > > does both A2 and e5500... > > Other than the toolchain issue, I'm not sure how this is worse than it > was before, when such a kernel would have had -Wa,-me500 forced. Probably similarly bad though it did work ... but if you are touching it, may as well do it right... > What -mcpu value should be used in such a combined kernel? Good question. We lack a generic booke option. What about powerpc64 ? A default like that is fine as long as tricky asm uses the macros for that and the *optional* -mcpu= option is available (and you can put it in defconfig). It might be worth asking gcc to add something like -march= or something like that though. > > I'm reverting that crap patch, please make such optimizations CONFIG_* > > options like power5...7 > > Speaking of crap patches, those config options don't limit themselves to > book3s and thus we're now getting CONFIG_GENERIC_CPU (and thus > -mtune=power7) on e5500 builds. Indeed, another crap patch ... I didn't spot it because it didn't break the build :-) Feel free to submit a fix ! Cheers, Ben. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] Revert "cxgb3: Check and handle the dma mapping errors"
On 08/14/2013 02:19 AM, Alexey Kardashevskiy wrote:
This reverts commit f83331bab149e29fa2c49cf102c0cd8c3f1ce9f9.
As the tests PPC64 (powernv platform) show, IOMMU pages are leaking
when transferring big amount of small packets (<=64 bytes),
"ping -f" and waiting for 15 seconds is the simplest way to confirm the bug.
Cc: Linus Torvalds
Cc: Santosh Rastapur
Cc: Jay Fenlason
Cc: David S. Miller
Cc: Divy Le ray
Signed-off-by: Alexey Kardashevskiy
Acked-by: Divy Le Ray
We are revisiting this patch in the light of the leak, and will repost
once fixed.
Cheers,
Divy
---
drivers/net/ethernet/chelsio/cxgb3/sge.c | 107 +++
1 file changed, 24 insertions(+), 83 deletions(-)
diff --git a/drivers/net/ethernet/chelsio/cxgb3/sge.c
b/drivers/net/ethernet/chelsio/cxgb3/sge.c
index 687ec4a..9c89dc8 100644
--- a/drivers/net/ethernet/chelsio/cxgb3/sge.c
+++ b/drivers/net/ethernet/chelsio/cxgb3/sge.c
@@ -455,11 +455,6 @@ static int alloc_pg_chunk(struct adapter *adapter, struct
sge_fl *q,
q->pg_chunk.offset = 0;
mapping = pci_map_page(adapter->pdev, q->pg_chunk.page,
0, q->alloc_size, PCI_DMA_FROMDEVICE);
- if (unlikely(pci_dma_mapping_error(adapter->pdev, mapping))) {
- __free_pages(q->pg_chunk.page, order);
- q->pg_chunk.page = NULL;
- return -EIO;
- }
q->pg_chunk.mapping = mapping;
}
sd->pg_chunk = q->pg_chunk;
@@ -954,75 +949,40 @@ static inline unsigned int calc_tx_descs(const struct
sk_buff *skb)
return flits_to_desc(flits);
}
-
-/* map_skb - map a packet main body and its page fragments
- * @pdev: the PCI device
- * @skb: the packet
- * @addr: placeholder to save the mapped addresses
- *
- * map the main body of an sk_buff and its page fragments, if any.
- */
-static int map_skb(struct pci_dev *pdev, const struct sk_buff *skb,
- dma_addr_t *addr)
-{
- const skb_frag_t *fp, *end;
- const struct skb_shared_info *si;
-
- *addr = pci_map_single(pdev, skb->data, skb_headlen(skb),
- PCI_DMA_TODEVICE);
- if (pci_dma_mapping_error(pdev, *addr))
- goto out_err;
-
- si = skb_shinfo(skb);
- end = &si->frags[si->nr_frags];
-
- for (fp = si->frags; fp < end; fp++) {
- *++addr = skb_frag_dma_map(&pdev->dev, fp, 0, skb_frag_size(fp),
- DMA_TO_DEVICE);
- if (pci_dma_mapping_error(pdev, *addr))
- goto unwind;
- }
- return 0;
-
-unwind:
- while (fp-- > si->frags)
- dma_unmap_page(&pdev->dev, *--addr, skb_frag_size(fp),
- DMA_TO_DEVICE);
-
- pci_unmap_single(pdev, addr[-1], skb_headlen(skb), PCI_DMA_TODEVICE);
-out_err:
- return -ENOMEM;
-}
-
/**
- * write_sgl - populate a scatter/gather list for a packet
+ * make_sgl - populate a scatter/gather list for a packet
*@skb: the packet
*@sgp: the SGL to populate
*@start: start address of skb main body data to include in the SGL
*@len: length of skb main body data to include in the SGL
- * @addr: the list of the mapped addresses
+ * @pdev: the PCI device
*
- * Copies the scatter/gather list for the buffers that make up a packet
+ * Generates a scatter/gather list for the buffers that make up a packet
*and returns the SGL size in 8-byte words. The caller must size the SGL
*appropriately.
*/
-static inline unsigned int write_sgl(const struct sk_buff *skb,
+static inline unsigned int make_sgl(const struct sk_buff *skb,
struct sg_ent *sgp, unsigned char *start,
- unsigned int len, const dma_addr_t *addr)
+ unsigned int len, struct pci_dev *pdev)
{
- unsigned int i, j = 0, k = 0, nfrags;
+ dma_addr_t mapping;
+ unsigned int i, j = 0, nfrags;
if (len) {
+ mapping = pci_map_single(pdev, start, len, PCI_DMA_TODEVICE);
sgp->len[0] = cpu_to_be32(len);
- sgp->addr[j++] = cpu_to_be64(addr[k++]);
+ sgp->addr[0] = cpu_to_be64(mapping);
+ j = 1;
}
nfrags = skb_shinfo(skb)->nr_frags;
for (i = 0; i < nfrags; i++) {
const skb_frag_t *frag = &skb_shinfo(skb)->frags[i];
+ mapping = skb_frag_dma_map(&pdev->dev, frag, 0, skb_frag_size(frag),
+ DMA_TO_DEVICE);
sgp->len[j] = cpu_to_be32(skb_frag_size(frag));
- sgp->addr[j] = cpu_to_be64(addr[k++]);
+ sgp->addr[j] = cpu_to_be64(mapping);
j ^= 1;
if (j == 0)
++sgp;
@@ -1178,7 +1138
Re: [GIT PULL] DT/core: cpu_ofnode updates for v3.12
On Wed, 2013-08-14 at 14:21 +0100, Sudeep KarkadaNagesha wrote: > IMO moving of handling ibm,ppc-interrupt-server#s to generic code > under > #ifdef CONFIG_PPC seems to be cleaner approach than weak definitation. > > As per my understanding each thread is a different logical cpu. > Each logical cpu is mapped to unique physical id(either present in reg > field or legacy ibm,ppc-interrupt-server#s field). So given a logical > cpu id we can get the cpu node corresponding to it. > Looking @ smp_setup_cpu_maps in arch/powerpc/kernel/setup-common.c > and the comment in the same file: "This implementation only supports > power of 2 number of threads.." the thread id id is implicit in the > logical cpu id. Do we need to fetch that from DT ? I don't want those parsing routines to make those assumptions. We have changed our logical numbering in the past and may again. Cheers, Ben. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH] crypto:nx - fix nx-aes-gcm verification
This patch fixes a bug in the nx-aes-gcm implementation.
Corrected the code so that the authtag is always verified after
decrypting and not just when there is associated data included.
Also, corrected the code to retrieve the input authtag from src
instead of dst.
Reviewed-by: Fionnuala Gunter
Reviewed-by: Marcelo Cerri
Signed-off-by: Joy Latten
diff --git a/drivers/crypto/nx/nx-aes-gcm.c b/drivers/crypto/nx/nx-aes-gcm.c
index 6cca6c3..eb851bb 100644
--- a/drivers/crypto/nx/nx-aes-gcm.c
+++ b/drivers/crypto/nx/nx-aes-gcm.c
@@ -243,11 +243,11 @@ static int gcm_aes_nx_crypt(struct aead_request *req, int
enc)
req->dst, nbytes,
crypto_aead_authsize(crypto_aead_reqtfm(req)),
SCATTERWALK_TO_SG);
- } else if (req->assoclen) {
+ } else {
u8 *itag = nx_ctx->priv.gcm.iauth_tag;
u8 *otag = csbcpb->cpb.aes_gcm.out_pat_or_mac;
- scatterwalk_map_and_copy(itag, req->dst, nbytes,
+ scatterwalk_map_and_copy(itag, req->src, nbytes,
crypto_aead_authsize(crypto_aead_reqtfm(req)),
SCATTERWALK_FROM_SG);
rc = memcmp(itag, otag,
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [RFC][PATCH] drivers: base: dynamic memory block creation
On Wed, Aug 14, 2013 at 04:52:53PM -0500, Seth Jennings wrote: > On Wed, Aug 14, 2013 at 02:37:26PM -0700, Yinghai Lu wrote: > > On Wed, Aug 14, 2013 at 1:35 PM, Greg Kroah-Hartman > > wrote: > > > On Wed, Aug 14, 2013 at 01:05:33PM -0700, Dave Hansen wrote: > > >> On 08/14/2013 12:43 PM, Greg Kroah-Hartman wrote: > > >> > On Wed, Aug 14, 2013 at 02:31:45PM -0500, Seth Jennings wrote: > > >> >> ppc64 has a normal memory block size of 256M (however sometimes as low > > >> >> as 16M depending on the system LMB size), and (I think) x86 is 128M. > > >> >> With > > >> >> 1TB of RAM and a 256M block size, that's 4k memory blocks with 20 > > >> >> sysfs > > >> >> entries per block that's around 80k items that need be created at boot > > >> >> time in sysfs. Some systems go up to 16TB where the issue is even > > >> >> more > > >> >> severe. > > >> > > > >> > The x86 developers are working with larger memory sizes and they > > >> > haven't > > >> > seen the problem in this area, for them it's in other places, as I > > >> > referred to in my other email. > > >> > > >> The SGI guys don't run normal distro kernels and don't turn on memory > > >> hotplug, so they don't see this. I do the same in my testing of > > >> large-memory x86 systems to speed up my boots. I'll go stick it back in > > >> there and see if I can generate some numbers for a 1TB machine. > > >> > > >> But, the problem on x86 is at _worst_ 1/8 of the problem on ppc64 since > > >> the SECTION_SIZE is so 8x bigger by default. > > >> > > >> Also, the cost of creating sections on ppc is *MUCH* higher than x86 > > >> when amortized across the number of pages that you're initializing. A > > >> section on ppc64 has to be created for each (2^24/2^16)=256 pages while > > >> one on x86 is created for each (2^27/2^12)=32768 pages. > > >> > > >> Thus, x86 folks with our small pages and large sections tend to be > > >> focused on per-page costs. The ppc folks with their small sections and > > >> larger pages tend to be focused on the per-section costs. > > > > > > Ah, thanks for the explaination, now it makes more sense why they are > > > both optimizing in different places. > > > > I had one local patch that sent before, it will probe block size for > > generic x86_64. > > set it to 2G looks more reasonable for system with 1T+ ram. > > If I am understanding you correctly, you are suggesting we make the block size > a boot time tunable. It can't be a runtime tunable since the memory blocks > are > currently created a boot time. > > On ppc64, we can't just just choose a memory block size since it must align > with the underlying LMB (logical memory block) size, set in the hardware ahead > of time. As long as the Linux block size is a multiple of the LMB size it should be possible. You'd just have to plug/unplug multiple LMBs at once. It would be possible to construct an LMB layout that defeats that, eg. every 2nd LMB not present, but I don't think that's actually a concern in practice. cheers ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [alsa-devel] [PATCH v4 1/2] ASoC: fsl: Add S/PDIF CPU DAI driver
Hi Stephen, On Wed, Aug 14, 2013 at 09:47:19AM -0600, Stephen Warren wrote: > If the clock source name list is different, then it needs a different > compatible value, so that each compatible value can specify which clock > names are required. > > Also, the compatible value itself should always include the exact HW > that's present (most specific HW version), as well as any other HW it's > compatible with. Thank you for the comments. Yes, I did so in v1-v3, but after rethinking about the situation (Actually both the HW version and the clock mux itself are same, just the clock sources connecting to the mux might be different), so I decided to do this by abstracting the driver from those source info and letting DT binding to pass such information. Because I think putting the clock sources into the driver differed by compatible value would make the driver more like SoC-specified, not the ideal way -- SoC-independent, since the clock sources are based on SoC design, not on itself. Thank you, Nicolin Chen ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v4 resent 1/2] ASoC: fsl: Add S/PDIF CPU DAI driver
Hi Philipp,
Thank again for the comments.
On Wed, Aug 14, 2013 at 02:06:24PM +0200, Philipp Zabel wrote:
> ==
> From i.MX53 reference manual:
>
> if (DPLL Locked) SPDIF_RxClk else extal
> 0001 if (DPLL Locked) SPDIF_RxClk else spdif_clk
> 0011 if (DPLL Locked) SPDIF_RxClk else asrc_clk
> 0100 if (DPLL Locked) SPDIF_Rxclk else esai_hckt
> 0101 extal_clk
> 0110 spdif_clk
> 1000 asrc_clk
> 1001 esai_hckt
> 1010 if (DPLL Locked) SPDIF_RxClk else mlb_clk
> 1011 if (DPLL Locked) SPDIF_RxClk else camp_clk
> 1100 mkb_clk
> 1101 camp_clk
> ==
>
> To me this looks like the device tree should just contain the list of
> unique clock inputs using phandles.
> /* for i.MX6Q: */
> clocks = <&...>;
> clock-names = "xtal", "spdif", "asrc",
> "spdif_ext", "esai", "mlb";
>
> /* for i.MX53: */
> clocks = <&...>;
> clock-names = "xtal", "spdif", "asrc",
> "esai", "mlb", "camp";
>
> The driver could contain this list of named inputs to the multiplexer
> and the DPLL locking information for each SoC version. The per-clock
> DPLL locking bit shouldn't be in the device tree at all.
I understand your point. I'll put the DPLL-locking info to the driver.
And I just found that the DPLL-lock condition seems to be fixed within
the range -- {0x0 ~ 0x4, 0xa ~ 0xb} (I checked i.MX6Q/6SL/53/35, all
of them are in such pattern.) So I don't need to put them into DT.
I'll revise it in next ver.
Thank you,
Nicolin Chen
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [alsa-devel] [PATCH v4 resent 1/2] ASoC: fsl: Add S/PDIF CPU DAI driver
On Wed, Aug 14, 2013 at 02:19:37PM +0200, Sascha Hauer wrote: > Yes, since the clk names are not an API. Exposing them to the devicetree > is not an option. The fact that the names are defined in > arch/arm/mach-imx/clk-imx6q.c and are used in the spdif driver makes > this really clear. > > The spdif core has 8 input clocks which have to be described in the > devicetree. Nobody says the mapping which clock name corresponds to > which bit combination has to be in the devicetree. Thank you for the explain. I get your point and really appreciate it. > Look at the possible clocks: > > if (DPLL Locked) SPDIF_RxClk else extal > 0001 if (DPLL Locked) SPDIF_RxClk else spdif_clk > 0010 if (DPLL Locked) SPDIF_RxClk else asrc_clk > 0011 if (DPLL Locked) SPDIF_RxClk else spdif_extclk > 0100 if (DPLL Locked) SPDIF_Rxclk else esai_hckt > 0101 extal_clk > 0110 spdif_clk > 0111 asrc_clk > 1000 spdif_extclk > 1001 esai_hckt > 1010 if (DPLL Locked) SPDIF_RxClk else mlb_clk > 1011 if (DPLL Locked) SPDIF_RxClk else mlb_phy_clk > 1100 mkb_clk > 1101 mlb_phy_clk > > Only half of them actually are clocks. "if (DPLL Locked) SPDIF_RxClk > else ..." is not a clock. Every sane hardware developer would have > introduced a mux with 8 entries and an additional "Use DPLL if possible" > bit. Now this is not the case here so we have to live with it and > maintain the above table in the driver. And another one for the i.MX35 > and still another one for i.MX53. I think I just have an idea for the table. I'll put them into the next version. Please take a look after I send it. Thank you, Nicolin Chen ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH v2] KVM: PPC: move iommu_add_device earlier
The current implementation of IOMMU on sPAPR does not use iommu_ops
and therefore does not call IOMMU API's bus_set_iommu() which
1) sets iommu_ops for a bus
2) registers a bus notifier
Instead, PCI devices are added to IOMMU groups from
subsys_initcall_sync(tce_iommu_init) which does basically the same
thing without using iommu_ops callbacks.
However Freescale PAMU driver (https://lkml.org/lkml/2013/7/1/158)
implements iommu_ops and when tce_iommu_init is called, every PCI device
is already added to some group so there is a conflict.
This patch does 2 things:
1. removes the loop in which PCI devices were added to groups and
adds devices as soon as they get the iommu_table pointer assigned to them.
For this, the set_iommu_table_base_and_group() function is introduced.
2. moves a bus notifier to powernv code (for hotplug) in order to avoid
conflict with the notifier from the Freescale driver.
iommu_add_device() and iommu_del_device() are public now.
Signed-off-by: Alexey Kardashevskiy
---
Changes:
v2:
* added a helper - set_iommu_table_base_and_group - which does
set_iommu_table_base() and iommu_add_device()
---
arch/powerpc/include/asm/iommu.h| 9 +++
arch/powerpc/kernel/iommu.c | 41 +++--
arch/powerpc/platforms/powernv/pci-ioda.c | 8 +++---
arch/powerpc/platforms/powernv/pci-p5ioc2.c | 2 +-
arch/powerpc/platforms/powernv/pci.c| 32 +-
arch/powerpc/platforms/pseries/iommu.c | 8 +++---
6 files changed, 54 insertions(+), 46 deletions(-)
diff --git a/arch/powerpc/include/asm/iommu.h b/arch/powerpc/include/asm/iommu.h
index c34656a..19ad77f 100644
--- a/arch/powerpc/include/asm/iommu.h
+++ b/arch/powerpc/include/asm/iommu.h
@@ -103,6 +103,15 @@ extern struct iommu_table *iommu_init_table(struct
iommu_table * tbl,
int nid);
extern void iommu_register_group(struct iommu_table *tbl,
int pci_domain_number, unsigned long pe_num);
+extern int iommu_add_device(struct device *dev);
+extern void iommu_del_device(struct device *dev);
+
+static inline void set_iommu_table_base_and_group(struct device *dev,
+ void *base)
+{
+ set_iommu_table_base(dev, base);
+ iommu_add_device(dev);
+}
extern int iommu_map_sg(struct device *dev, struct iommu_table *tbl,
struct scatterlist *sglist, int nelems,
diff --git a/arch/powerpc/kernel/iommu.c b/arch/powerpc/kernel/iommu.c
index b20ff17..15f8ca8 100644
--- a/arch/powerpc/kernel/iommu.c
+++ b/arch/powerpc/kernel/iommu.c
@@ -1105,7 +1105,7 @@ void iommu_release_ownership(struct iommu_table *tbl)
}
EXPORT_SYMBOL_GPL(iommu_release_ownership);
-static int iommu_add_device(struct device *dev)
+int iommu_add_device(struct device *dev)
{
struct iommu_table *tbl;
int ret = 0;
@@ -1134,46 +1134,13 @@ static int iommu_add_device(struct device *dev)
return ret;
}
+EXPORT_SYMBOL_GPL(iommu_add_device);
-static void iommu_del_device(struct device *dev)
+void iommu_del_device(struct device *dev)
{
iommu_group_remove_device(dev);
}
-
-static int iommu_bus_notifier(struct notifier_block *nb,
- unsigned long action, void *data)
-{
- struct device *dev = data;
-
- switch (action) {
- case BUS_NOTIFY_ADD_DEVICE:
- return iommu_add_device(dev);
- case BUS_NOTIFY_DEL_DEVICE:
- iommu_del_device(dev);
- return 0;
- default:
- return 0;
- }
-}
-
-static struct notifier_block tce_iommu_bus_nb = {
- .notifier_call = iommu_bus_notifier,
-};
-
-static int __init tce_iommu_init(void)
-{
- struct pci_dev *pdev = NULL;
-
- BUILD_BUG_ON(PAGE_SIZE < IOMMU_PAGE_SIZE);
-
- for_each_pci_dev(pdev)
- iommu_add_device(&pdev->dev);
-
- bus_register_notifier(&pci_bus_type, &tce_iommu_bus_nb);
- return 0;
-}
-
-subsys_initcall_sync(tce_iommu_init);
+EXPORT_SYMBOL_GPL(iommu_del_device);
#else
diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c
b/arch/powerpc/platforms/powernv/pci-ioda.c
index d8140b1..756bb58 100644
--- a/arch/powerpc/platforms/powernv/pci-ioda.c
+++ b/arch/powerpc/platforms/powernv/pci-ioda.c
@@ -440,7 +440,7 @@ static void pnv_pci_ioda_dma_dev_setup(struct pnv_phb *phb,
struct pci_dev *pdev
return;
pe = &phb->ioda.pe_array[pdn->pe_number];
- set_iommu_table_base(&pdev->dev, &pe->tce32_table);
+ set_iommu_table_base_and_group(&pdev->dev, &pe->tce32_table);
}
static void pnv_ioda_setup_bus_dma(struct pnv_ioda_pe *pe, struct pci_bus *bus)
@@ -448,7 +448,7 @@ static void pnv_ioda_setup_bus_dma(struct pnv_ioda_pe *pe,
struct pci_bus *bus)
struct pci_dev *dev;
list_for_each_entry(dev, &bus->devices, bus_list) {
- set_iommu_table_base(&dev->dev, &pe->tc
[PATCH 1/6] powerpc: Update the 00-Index in Documentation/powerpc
People have been dropping things in here without updating the index, do it for them. Signed-off-by: Michael Ellerman --- Documentation/powerpc/00-INDEX | 11 +++ 1 file changed, 11 insertions(+) diff --git a/Documentation/powerpc/00-INDEX b/Documentation/powerpc/00-INDEX index 05026ce..6db73df 100644 --- a/Documentation/powerpc/00-INDEX +++ b/Documentation/powerpc/00-INDEX @@ -5,13 +5,20 @@ please mail me. 00-INDEX - this file +bootwrapper.txt + - Information on how the powerpc kernel is wrapped for boot on various + different platforms. cpu_features.txt - info on how we support a variety of CPUs with minimal compile-time options. eeh-pci-error-recovery.txt - info on PCI Bus EEH Error Recovery +firmware-assisted-dump.txt + - Documentation on the firmware assisted dump mechanism "fadump". hvcs.txt - IBM "Hypervisor Virtual Console Server" Installation Guide +kvm_440.txt + - Various notes on the implementation of KVM for PowerPC 440. mpc52xx.txt - Linux 2.6.x on MPC52xx family pmu-ebb.txt @@ -19,3 +26,7 @@ pmu-ebb.txt qe_firmware.txt - describes the layout of firmware binaries for the Freescale QUICC Engine and the code that parses and uploads the microcode therein. +ptrace.txt + - Information on the ptrace interfaces for hardware debug registers. +transactional_memory.txt + - Overview of the Power8 transactional memory support. -- 1.8.1.2 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 2/6] powerpc/pseries: Add a warning in the case of cross-cpu VPA registration
The spec says it "may be problematic" if CPU x registers the VPA of CPU y. Add a warning in case we ever do that. Signed-off-by: Michael Ellerman --- arch/powerpc/platforms/pseries/lpar.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/arch/powerpc/platforms/pseries/lpar.c b/arch/powerpc/platforms/pseries/lpar.c index 8bad880..73d6c1b 100644 --- a/arch/powerpc/platforms/pseries/lpar.c +++ b/arch/powerpc/platforms/pseries/lpar.c @@ -68,6 +68,12 @@ void vpa_init(int cpu) struct paca_struct *pp; struct dtl_entry *dtl; + /* +* The spec says it "may be problematic" if CPU x registers the VPA of +* CPU y. We should never do that, but wail if we ever do. +*/ + WARN_ON(cpu != smp_processor_id()); + if (cpu_has_feature(CPU_FTR_ALTIVEC)) lppaca_of(cpu).vmxregs_in_use = 1; -- 1.8.1.2 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 3/6] powerpc: Add more trap names to xmon
We haven't updated these for a while it seems, it's nice to have in the oops output. Signed-off-by: Michael Ellerman --- arch/powerpc/xmon/xmon.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c index 96bf5bd..9f3655b 100644 --- a/arch/powerpc/xmon/xmon.c +++ b/arch/powerpc/xmon/xmon.c @@ -1256,11 +1256,18 @@ const char *getvecname(unsigned long vec) case 0x700: ret = "(Program Check)"; break; case 0x800: ret = "(FPU Unavailable)"; break; case 0x900: ret = "(Decrementer)"; break; + case 0x980: ret = "(Hypervisor Decrementer)"; break; + case 0xa00: ret = "(Doorbell)"; break; case 0xc00: ret = "(System Call)"; break; case 0xd00: ret = "(Single Step)"; break; + case 0xe40: ret = "(Emulation Assist)"; break; + case 0xe60: ret = "(HMI)"; break; + case 0xe80: ret = "(Hypervisor Doorbell)"; break; case 0xf00: ret = "(Performance Monitor)"; break; case 0xf20: ret = "(Altivec Unavailable)"; break; case 0x1300:ret = "(Instruction Breakpoint)"; break; + case 0x1500:ret = "(Denormalisation)"; break; + case 0x1700:ret = "(Altivec Assist)"; break; default: ret = ""; } return ret; -- 1.8.1.2 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 4/6] powerpc: Fix location and rename exception trampolines
The symbols that name some of our exception trampolines are ahead of the location they name. In most cases this is OK because the code is tightly packed, but in some cases it means the symbol floats ahead of the correct location, eg: cea0 : ... cf00: 7d b2 43 a6 mtsprg 2,r13 Fix them all by moving the symbol after the set of the location. While we're moving them anyway, rename them to loose the camelcase and to make it clear that they are trampolines. Signed-off-by: Michael Ellerman --- arch/powerpc/kernel/exceptions-64s.S | 20 ++-- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S index 902ca3c..ae902cae 100644 --- a/arch/powerpc/kernel/exceptions-64s.S +++ b/arch/powerpc/kernel/exceptions-64s.S @@ -323,32 +323,32 @@ hv_exception_trampoline: * prolog code of the PerformanceMonitor one. A little * trickery is thus necessary */ -performance_monitor_pSeries_1: . = 0xf00 +performance_monitor_pseries_trampoline: SET_SCRATCH0(r13) EXCEPTION_PROLOG_0(PACA_EXGEN) b performance_monitor_pSeries -altivec_unavailable_pSeries_1: . = 0xf20 +altivec_unavailable_pseries_trampoline: SET_SCRATCH0(r13) EXCEPTION_PROLOG_0(PACA_EXGEN) b altivec_unavailable_pSeries -vsx_unavailable_pSeries_1: . = 0xf40 +vsx_unavailable_pseries_trampoline: SET_SCRATCH0(r13) EXCEPTION_PROLOG_0(PACA_EXGEN) b vsx_unavailable_pSeries -facility_unavailable_trampoline: . = 0xf60 +facility_unavailable_trampoline: SET_SCRATCH0(r13) EXCEPTION_PROLOG_0(PACA_EXGEN) b facility_unavailable_pSeries -hv_facility_unavailable_trampoline: . = 0xf80 +hv_facility_unavailable_trampoline: SET_SCRATCH0(r13) EXCEPTION_PROLOG_0(PACA_EXGEN) b facility_unavailable_hv @@ -820,32 +820,32 @@ system_call_relon_pSeries: EXCEPTION_PROLOG_0(PACA_EXGEN) b h_doorbell_relon_hv -performance_monitor_relon_pSeries_1: . = 0x4f00 +performance_monitor_relon_pseries_trampoline: SET_SCRATCH0(r13) EXCEPTION_PROLOG_0(PACA_EXGEN) b performance_monitor_relon_pSeries -altivec_unavailable_relon_pSeries_1: . = 0x4f20 +altivec_unavailable_relon_pseries_trampoline: SET_SCRATCH0(r13) EXCEPTION_PROLOG_0(PACA_EXGEN) b altivec_unavailable_relon_pSeries -vsx_unavailable_relon_pSeries_1: . = 0x4f40 +vsx_unavailable_relon_pseries_trampoline: SET_SCRATCH0(r13) EXCEPTION_PROLOG_0(PACA_EXGEN) b vsx_unavailable_relon_pSeries -facility_unavailable_relon_trampoline: . = 0x4f60 +facility_unavailable_relon_trampoline: SET_SCRATCH0(r13) EXCEPTION_PROLOG_0(PACA_EXGEN) b facility_unavailable_relon_pSeries -hv_facility_unavailable_relon_trampoline: . = 0x4f80 +hv_facility_unavailable_relon_trampoline: SET_SCRATCH0(r13) EXCEPTION_PROLOG_0(PACA_EXGEN) b hv_facility_unavailable_relon_hv -- 1.8.1.2 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 5/6] powerpc: Add more exception trampolines for hypervisor exceptions
This makes back traces and profiles easier to read. Signed-off-by: Michael Ellerman --- arch/powerpc/kernel/exceptions-64s.S | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S index ae902cae..e41f066 100644 --- a/arch/powerpc/kernel/exceptions-64s.S +++ b/arch/powerpc/kernel/exceptions-64s.S @@ -293,27 +293,31 @@ system_call_pSeries: * out of line to handle them */ . = 0xe00 -hv_exception_trampoline: +hv_data_storage_trampoline: SET_SCRATCH0(r13) EXCEPTION_PROLOG_0(PACA_EXGEN) b h_data_storage_hv . = 0xe20 +hv_instr_storage_trampoline: SET_SCRATCH0(r13) EXCEPTION_PROLOG_0(PACA_EXGEN) b h_instr_storage_hv . = 0xe40 +emulation_assist_trampoline: SET_SCRATCH0(r13) EXCEPTION_PROLOG_0(PACA_EXGEN) b emulation_assist_hv . = 0xe60 +hv_exception_trampoline: SET_SCRATCH0(r13) EXCEPTION_PROLOG_0(PACA_EXGEN) b hmi_exception_hv . = 0xe80 +hv_doorbell_trampoline: SET_SCRATCH0(r13) EXCEPTION_PROLOG_0(PACA_EXGEN) b h_doorbell_hv @@ -808,6 +812,7 @@ system_call_relon_pSeries: b . /* Can't happen, see v2.07 Book III-S section 6.5 */ . = 0x4e40 +emulation_assist_relon_trampoline: SET_SCRATCH0(r13) EXCEPTION_PROLOG_0(PACA_EXGEN) b emulation_assist_relon_hv @@ -816,6 +821,7 @@ system_call_relon_pSeries: b . /* Can't happen, see v2.07 Book III-S section 6.5 */ . = 0x4e80 +h_doorbell_relon_trampoline: SET_SCRATCH0(r13) EXCEPTION_PROLOG_0(PACA_EXGEN) b h_doorbell_relon_hv -- 1.8.1.2 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 6/6] powerpc: Skip emulating & leave interrupts off for kernel program checks
In the program check handler we handle some causes with interrupts off and others with interrupts on. We need to enable interrupts to handle the emulation cases, because they access userspace memory and might sleep. For faults in the kernel we don't want to do any emulation, and emulate_instruction() enforces that. do_mathemu() doesn't but probably should. The other disadvantage of enabling interrupts for kernel faults is that we may take another interrupt, and recurse. As seen below: --- Exception: e40 at c0004ee0 performance_monitor_relon_pSeries_1 [link register ] c000f858 .arch_local_irq_restore+0x38/0x90 [c00fb185dc10] (unreliable) [c00fb185dc80] c07d8558 .program_check_exception+0x298/0x2d0 [c00fb185dd00] c0002f40 emulation_assist_common+0x140/0x180 --- Exception: e40 at c0004ee0 performance_monitor_relon_pSeries_1 [link register ] c000f858 .arch_local_irq_restore+0x38/0x90 [c00fb185dff0] 008b9190 (unreliable) [c00fb185e060] c07d8558 .program_check_exception+0x298/0x2d0 So avoid both problems by checking if the fault was in the kernel and skipping the enable of interrupts and the emulation. Go straight to delivering the SIGILL, which for kernel faults calls die() and so on, dropping us in the debugger etc. Signed-off-by: Michael Ellerman --- arch/powerpc/kernel/traps.c | 11 +++ 1 file changed, 11 insertions(+) diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c index e435bc0..9515bde 100644 --- a/arch/powerpc/kernel/traps.c +++ b/arch/powerpc/kernel/traps.c @@ -1116,6 +1116,16 @@ void __kprobes program_check_exception(struct pt_regs *regs) } #endif + /* +* If we took the program check in the kernel skip down to sending a +* SIGILL. The subsequent cases all relate to emulating instructions +* which we should only do for userspace. We also do not want to enable +* interrupts for kernel faults because that might lead to further +* faults, and loose the context of the original exception. +*/ + if (!user_mode(regs)) + goto sigill; + /* We restore the interrupt state now */ if (!arch_irq_disabled_regs(regs)) local_irq_enable(); @@ -1168,6 +1178,7 @@ void __kprobes program_check_exception(struct pt_regs *regs) } } +sigill: if (reason & REASON_PRIVILEGED) _exception(SIGILL, regs, ILL_PRVOPC, regs->nip); else -- 1.8.1.2 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 2/8] powerpc/pseries: fix over writing of rtas return code in update_dt_node
The rc variable is initially used to store the return code from the
ibm,update-properties rtas call which returns 0 or 1 on success. A return
code of 1 indicates that ibm,update-properties must be called again for the
node. However, the rc variable is overwritten by a call to update_dt_prop
which returns 0 on success. This results in ibm,update-properties not being
called again for the given node when the rtas call rc was previously 1.
Signed-off-by: Tyrel Datwyler
---
arch/powerpc/platforms/pseries/mobility.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/mobility.c
b/arch/powerpc/platforms/pseries/mobility.c
index f28abee..aaae85d 100644
--- a/arch/powerpc/platforms/pseries/mobility.c
+++ b/arch/powerpc/platforms/pseries/mobility.c
@@ -130,7 +130,7 @@ static int update_dt_node(u32 phandle, s32 scope)
struct update_props_workarea *upwa;
struct device_node *dn;
struct property *prop = NULL;
- int i, rc;
+ int i, rc, rtas_rc;
char *prop_data;
char *rtas_buf;
int update_properties_token;
@@ -154,9 +154,9 @@ static int update_dt_node(u32 phandle, s32 scope)
upwa->phandle = phandle;
do {
- rc = mobility_rtas_call(update_properties_token, rtas_buf,
+ rtas_rc = mobility_rtas_call(update_properties_token, rtas_buf,
scope);
- if (rc < 0)
+ if (rtas_rc < 0)
break;
prop_data = rtas_buf + sizeof(*upwa);
@@ -202,7 +202,7 @@ static int update_dt_node(u32 phandle, s32 scope)
prop_data += vd;
}
}
- } while (rc == 1);
+ } while (rtas_rc == 1);
of_node_put(dn);
kfree(rtas_buf);
--
1.7.12.4
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 1/8] powerpc/pseries: fix creation of loop in device node property list
The update_dt_prop helper function fails to set the IN/OUT parameter prop to
NULL after a complete property has been parsed from the work area returned by
the ibm,update-properties rtas function. This results in the property list of
the device node being updated is corrupted and becomes a loop since the same
property structure is used repeatedly.
Signed-off-by: Tyrel Datwyler
---
arch/powerpc/platforms/pseries/mobility.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/platforms/pseries/mobility.c
b/arch/powerpc/platforms/pseries/mobility.c
index 3d01eee..f28abee 100644
--- a/arch/powerpc/platforms/pseries/mobility.c
+++ b/arch/powerpc/platforms/pseries/mobility.c
@@ -119,7 +119,7 @@ static int update_dt_property(struct device_node *dn,
struct property **prop,
if (!more) {
of_update_property(dn, new_prop);
- new_prop = NULL;
+ *prop = NULL;
}
return 0;
--
1.7.12.4
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 3/8] powerpc/pseries: pack update_props_workarea to map correctly to rtas buffer header
The work area buffer returned by the ibm,update-properties rtas call contains
20 bytes of header information prior to the property value descriptor data.
Currently update_dt_node tries to advance over this header using sizeof(upwa).
The update_props_workarea struct contains 20 bytes worth of fields, that map
to the relevant header data, but the sizeof the structure is 24 bytes due to
4 bytes of padding at the end of the structure. Packing the structure ensures
that we don't advance too far over the rtas buffer.
Signed-off-by: Tyrel Datwyler
---
arch/powerpc/platforms/pseries/mobility.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/platforms/pseries/mobility.c
b/arch/powerpc/platforms/pseries/mobility.c
index aaae85d..023e354 100644
--- a/arch/powerpc/platforms/pseries/mobility.c
+++ b/arch/powerpc/platforms/pseries/mobility.c
@@ -28,7 +28,7 @@ struct update_props_workarea {
u32 state;
u64 reserved;
u32 nprops;
-};
+} __packed;
#define NODE_ACTION_MASK 0xff00
#define NODE_COUNT_MASK0x00ff
--
1.7.12.4
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 7/8] powerpc/pseries: add mising of_node_put in delete_dt_node
The node to be detached is retrieved via its phandle by a call to of_find_node_by_phandle which increments the ref count. We need a matching call to of_node_put to decrement the ref count and ensure the node is actually freed. Signed-off-by: Tyrel Datwyler --- arch/powerpc/platforms/pseries/mobility.c | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/powerpc/platforms/pseries/mobility.c b/arch/powerpc/platforms/pseries/mobility.c index ff102e2..cde4e0a 100644 --- a/arch/powerpc/platforms/pseries/mobility.c +++ b/arch/powerpc/platforms/pseries/mobility.c @@ -62,6 +62,7 @@ static int delete_dt_node(u32 phandle) return -ENOENT; dlpar_detach_node(dn); + of_node_put(dn); return 0; } -- 1.7.12.4 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 0/8] powerpc/pseries: fix/cleanup broken mobility device-tree update code
Currently we still perform updating of the device tree after a suspend or migration from drmgr in userspace through the ugly /proc/ppc64/ofdt interface. Code exists to do this update from within the kernel, but testing revealed it to be severely broken to the point that the system hardlocks up stuck in an infinite loop with irqs disabled. The first 4 patches in the series fix several issues encountered tyring to perform node property updates as dictated by the ibm,update-properties rtas call. The next 2 patches refactor the logic of dlpar_configure_connector to enusre that nodes created in a device nodes subtree are initialized correctly. The remainder of the patchset fixes a couple problems with node removal. Tyrel Datwyler (8): powerpc/pseries: fix creation of loop in device node property list powerpc/pseries: fix over writing of rtas return code in update_dt_node powerpc/pseries: pack update_props_workarea to map correctly to rtas buffer header powerpc/pseries: fix parsing of initial node path in update_dt_node powerpc/pseries: do all node initialization in dlpar_parse_cc_node powerpc/pseries: make dlpar_configure_connector parent node aware powerpc/pseries: add mising of_node_put in delete_dt_node powerpc/pseries: child nodes are not detached by dlpar_detach_node arch/powerpc/platforms/pseries/dlpar.c| 67 ++- arch/powerpc/platforms/pseries/mobility.c | 45 ++--- arch/powerpc/platforms/pseries/pseries.h | 2 +- 3 files changed, 62 insertions(+), 52 deletions(-) -- 1.7.12.4 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 4/8] powerpc/pseries: fix parsing of initial node path in update_dt_node
On the first call to ibm,update-properties for a node the first property
returned is the full node path. Currently this is not parsed correctly by the
update_dt_node function. Commit 2e9b7b0 attempted to fix this, but was
incorrect as it made a wrong assumption about the layout of the first
property in the work area. Further, if ibm,update-properties must be called
multiple times for the same node this special property should only be skipped
after the initial call. The first property descriptor returned consists of
the property name, property value length, and property value. The property
name is an empty string, property length is encoded in 4 byte integer, and
the property value is the node path.
Signed-off-by: Tyrel Datwyler
---
arch/powerpc/platforms/pseries/mobility.c | 21 +++--
1 file changed, 11 insertions(+), 10 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/mobility.c
b/arch/powerpc/platforms/pseries/mobility.c
index 023e354..ed5426f 100644
--- a/arch/powerpc/platforms/pseries/mobility.c
+++ b/arch/powerpc/platforms/pseries/mobility.c
@@ -161,18 +161,19 @@ static int update_dt_node(u32 phandle, s32 scope)
prop_data = rtas_buf + sizeof(*upwa);
- /* The first element of the buffer is the path of the node
-* being updated in the form of a 8 byte string length
-* followed by the string. Skip past this to get to the
-* properties being updated.
+ /* On the first call to ibm,update-properties for a node the
+* the first property value descriptor contains an empty
+* property name, the property value length encoded as u32,
+* and the property value is the node path being updated.
*/
- vd = *prop_data++;
- prop_data += vd;
+ if (*prop_data == 0) {
+ prop_data++;
+ vd = *(u32 *)prop_data;
+ prop_data += vd + sizeof(vd);
+ upwa->nprops--;
+ }
- /* The path we skipped over is counted as one of the elements
-* returned so start counting at one.
-*/
- for (i = 1; i < upwa->nprops; i++) {
+ for (i = 0; i < upwa->nprops; i++) {
char *prop_name;
prop_name = prop_data;
--
1.7.12.4
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 6/8] powerpc/pseries: make dlpar_configure_connector parent node aware
Currently the device nodes created in the device subtree returned by a call to
dlpar_configure_connector are all named in the root node. This is because the
the node name in the work area returned by ibm,configure-connector rtas call
only contains the node name and not the entire node path. Passing the parent
node where the new subtree will be created to dlpar_configure_connector allows
the correct node path to be prefixed in the full_name field.
Signed-off-by: Tyrel Datwyler
---
arch/powerpc/platforms/pseries/dlpar.c| 55 ---
arch/powerpc/platforms/pseries/mobility.c | 11 +++
arch/powerpc/platforms/pseries/pseries.h | 2 +-
3 files changed, 34 insertions(+), 34 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/dlpar.c
b/arch/powerpc/platforms/pseries/dlpar.c
index c855233..4ea667d 100644
--- a/arch/powerpc/platforms/pseries/dlpar.c
+++ b/arch/powerpc/platforms/pseries/dlpar.c
@@ -63,21 +63,24 @@ static struct property *dlpar_parse_cc_property(struct
cc_workarea *ccwa)
return prop;
}
-static struct device_node *dlpar_parse_cc_node(struct cc_workarea *ccwa)
+static struct device_node *dlpar_parse_cc_node(struct cc_workarea *ccwa,
+ const char *path)
{
struct device_node *dn;
char *name;
+ /* If parent node path is "/" advance path to NULL terminator to
+* prevent double leading slashs in full_name.
+*/
+ if (!path[1])
+ path++;
+
dn = kzalloc(sizeof(*dn), GFP_KERNEL);
if (!dn)
return NULL;
- /* The configure connector reported name does not contain a
-* preceding '/', so we allocate a buffer large enough to
-* prepend this to the full_name.
-*/
name = (char *)ccwa + ccwa->name_offset;
- dn->full_name = kasprintf(GFP_KERNEL, "/%s", name);
+ dn->full_name = kasprintf(GFP_KERNEL, "%s/%s", path, name);
if (!dn->full_name) {
kfree(dn);
return NULL;
@@ -123,7 +126,8 @@ void dlpar_free_cc_nodes(struct device_node *dn)
#define CALL_AGAIN -2
#define ERR_CFG_USE -9003
-struct device_node *dlpar_configure_connector(u32 drc_index)
+struct device_node *dlpar_configure_connector(u32 drc_index,
+ struct device_node *parent)
{
struct device_node *dn;
struct device_node *first_dn = NULL;
@@ -132,6 +136,7 @@ struct device_node *dlpar_configure_connector(u32 drc_index)
struct property *last_property = NULL;
struct cc_workarea *ccwa;
char *data_buf;
+ const char *parent_path = parent->full_name;
int cc_token;
int rc = -1;
@@ -165,7 +170,7 @@ struct device_node *dlpar_configure_connector(u32 drc_index)
break;
case NEXT_SIBLING:
- dn = dlpar_parse_cc_node(ccwa);
+ dn = dlpar_parse_cc_node(ccwa, parent_path);
if (!dn)
goto cc_error;
@@ -175,13 +180,17 @@ struct device_node *dlpar_configure_connector(u32
drc_index)
break;
case NEXT_CHILD:
- dn = dlpar_parse_cc_node(ccwa);
+ if (first_dn)
+ parent_path = last_dn->full_name;
+
+ dn = dlpar_parse_cc_node(ccwa, parent_path);
if (!dn)
goto cc_error;
- if (!first_dn)
+ if (!first_dn) {
+ dn->parent = parent;
first_dn = dn;
- else {
+ } else {
dn->parent = last_dn;
if (last_dn)
last_dn->child = dn;
@@ -205,6 +214,7 @@ struct device_node *dlpar_configure_connector(u32 drc_index)
case PREV_PARENT:
last_dn = last_dn->parent;
+ parent_path = last_dn->parent->full_name;
break;
case CALL_AGAIN:
@@ -383,9 +393,8 @@ out:
static ssize_t dlpar_cpu_probe(const char *buf, size_t count)
{
- struct device_node *dn;
+ struct device_node *dn, *parent;
unsigned long drc_index;
- char *cpu_name;
int rc;
cpu_hotplug_driver_lock();
@@ -395,25 +404,19 @@ static ssize_t dlpar_cpu_probe(const char *buf, size_t
count)
goto out;
}
- dn = dlpar_configure_connector(drc_index);
- if (!dn) {
- rc = -EINVAL;
+ parent = of_find_node_by_path("/cpus");
+ if (!parent) {
+ rc = -ENODEV;
goto out;
}
- /* configure-connector reports cpus as living in the base
-* dire
[PATCH 5/8] powerpc/pseries: do all node initialization in dlpar_parse_cc_node
Currently the OF_DYNAMIC and kref initialization for a node happens in
dlpar_attach_node. However, a node passed to dlpar_attach_node may be a tree
containing child nodes, and no initialization traversal is done on the
tree. Since the children never get their kref initialized or the OF_DYNAMIC
flag set these nodes are prevented from ever being released from memory
should they become detached. This initialization step is better done at the
time each node is allocated in dlpar_parse_cc_node.
Signed-off-by: Tyrel Datwyler
---
arch/powerpc/platforms/pseries/dlpar.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/dlpar.c
b/arch/powerpc/platforms/pseries/dlpar.c
index a1a7b9a..c855233 100644
--- a/arch/powerpc/platforms/pseries/dlpar.c
+++ b/arch/powerpc/platforms/pseries/dlpar.c
@@ -83,6 +83,9 @@ static struct device_node *dlpar_parse_cc_node(struct
cc_workarea *ccwa)
return NULL;
}
+ of_node_set_flag(dn, OF_DYNAMIC);
+ kref_init(&dn->kref);
+
return dn;
}
@@ -256,8 +259,6 @@ int dlpar_attach_node(struct device_node *dn)
{
int rc;
- of_node_set_flag(dn, OF_DYNAMIC);
- kref_init(&dn->kref);
dn->parent = derive_parent(dn->full_name);
if (!dn->parent)
return -ENOMEM;
--
1.7.12.4
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 8/8] powerpc/pseries: child nodes are not detached by dlpar_detach_node
Calls to dlpar_detach_node do not iterate over child nodes detaching them as
well. By iterating and detaching the child nodes we ensure that they have the
OF_DETACHED flag set and that their reference counts are decremented such that
the node will be freed from memory by of_node_release.
Signed-off-by: Tyrel Datwyler
---
arch/powerpc/platforms/pseries/dlpar.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/arch/powerpc/platforms/pseries/dlpar.c
b/arch/powerpc/platforms/pseries/dlpar.c
index 4ea667d..7cfdaae 100644
--- a/arch/powerpc/platforms/pseries/dlpar.c
+++ b/arch/powerpc/platforms/pseries/dlpar.c
@@ -286,8 +286,15 @@ int dlpar_attach_node(struct device_node *dn)
int dlpar_detach_node(struct device_node *dn)
{
+ struct device_node *child;
int rc;
+ child = of_get_next_child(dn, NULL);
+ while (child) {
+ dlpar_detach_node(child);
+ child = of_get_next_child(dn, child);
+ }
+
rc = of_detach_node(dn);
if (rc)
return rc;
--
1.7.12.4
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v2] KVM: PPC: move iommu_add_device earlier
Alexey,
On Thu, Aug 15, 2013 at 12:57:19PM +1000, Alexey Kardashevskiy wrote:
>The current implementation of IOMMU on sPAPR does not use iommu_ops
>and therefore does not call IOMMU API's bus_set_iommu() which
>1) sets iommu_ops for a bus
>2) registers a bus notifier
>Instead, PCI devices are added to IOMMU groups from
>subsys_initcall_sync(tce_iommu_init) which does basically the same
>thing without using iommu_ops callbacks.
>
>However Freescale PAMU driver (https://lkml.org/lkml/2013/7/1/158)
>implements iommu_ops and when tce_iommu_init is called, every PCI device
>is already added to some group so there is a conflict.
>
>This patch does 2 things:
>1. removes the loop in which PCI devices were added to groups and
>adds devices as soon as they get the iommu_table pointer assigned to them.
>For this, the set_iommu_table_base_and_group() function is introduced.
>2. moves a bus notifier to powernv code (for hotplug) in order to avoid
>conflict with the notifier from the Freescale driver.
>
>iommu_add_device() and iommu_del_device() are public now.
Small suggestion, how about add a prefix like "ppc_"?
Since on intel, it has intel_iommu_add_device. Maybe this could help the
audience.
>
>Signed-off-by: Alexey Kardashevskiy
>---
>
>@@ -623,3 +623,33 @@ void __init pnv_pci_init(void)
> ppc_md.teardown_msi_irqs = pnv_teardown_msi_irqs;
> #endif
> }
>+
>+static int tce_iommu_bus_notifier(struct notifier_block *nb,
>+ unsigned long action, void *data)
>+{
>+ struct device *dev = data;
>+
>+ switch (action) {
>+ case BUS_NOTIFY_ADD_DEVICE:
>+ return iommu_add_device(dev);
>+ case BUS_NOTIFY_DEL_DEVICE:
>+ iommu_del_device(dev);
>+ return 0;
Recently, I encounter a problem for device remove. In some cases, the device
will not belong to any iommu_group. For example, the DMA space is not enough
and can't allocate a TCE segment. (This happens on P7IOC. I think on P8 it
won't happen.) In this case, dev->iommu_group would be NULL and kernel crash
in iommu_group_remove_device(), since it try to reference group->notifier.
In iommu_bus_notifier(), it will check dev->iommu_group before calling the
remove_device.
if (ops->remove_device && dev->iommu_group)
So I suggest to add this check here too.
BTW, I have a patch like this, which I put the check in
iommu_group_remove_device.
This could protect the kernel from do the removing without the check outside.
Author: Wei Yang
Date: Wed Aug 14 04:45:06 2013 -0400
iommu: check dev->iommu_group before removing a device
In some cases, one device may not associate with any iommu_group.
For example, not enough DMA address space.
For those devices, kernel will crash when try to remove it from an
iommu_group.
This patch do the check before remove it.
Signed-off-by: Wei Yang
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index fbe9ca7..fe41946 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -379,6 +379,9 @@ void iommu_group_remove_device(struct device *dev)
struct iommu_group *group = dev->iommu_group;
struct iommu_device *tmp_device, *device = NULL;
+ if (!group)
+ return;
+
/* Pre-notify listeners that a device is being removed. */
blocking_notifier_call_chain(&group->notifier,
IOMMU_GROUP_NOTIFY_DEL_DEVICE, dev);
I am not sure which place is better, in iommu_group_remove_device() or in the
tce_iommu_bus_notifier().
I am glad to hear your suggestions.
--
Richard Yang
Help you, Help me
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
