* [PATCH 2/5] ASoC: fsl: mpc5200 combine psc_dma platform data
From: Eric Millbrandt @ 2012-09-12 2:14 UTC (permalink / raw)
To: Grant Likely, Liam Girdwood, Mark Brown, Anatolij Gustschin
Cc: alsa-devel, linuxppc-dev, Eric Millbrandt
In-Reply-To: <1347416089-23393-1-git-send-email-emillbrandt@dekaresearch.com>
The mpc5200_psc_ac97 and mpc5200_psc_i2s modules rely on shared platform data
with mpc5200_dma.
Signed-off-by: Eric Millbrandt <emillbrandt@dekaresearch.com>
---
sound/soc/fsl/mpc5200_dma.c | 24 ++++--------------------
sound/soc/fsl/mpc5200_dma.h | 3 +++
sound/soc/fsl/mpc5200_psc_ac97.c | 5 +++++
sound/soc/fsl/mpc5200_psc_i2s.c | 5 +++++
4 files changed, 17 insertions(+), 20 deletions(-)
diff --git a/sound/soc/fsl/mpc5200_dma.c b/sound/soc/fsl/mpc5200_dma.c
index 46fb518..4fabc85 100644
--- a/sound/soc/fsl/mpc5200_dma.c
+++ b/sound/soc/fsl/mpc5200_dma.c
@@ -370,7 +370,7 @@ static struct snd_soc_platform_driver mpc5200_audio_dma_platform = {
.pcm_free = psc_dma_free,
};
-static int mpc5200_hpcd_probe(struct platform_device *op)
+int mpc5200_audio_dma_create(struct platform_device *op)
{
phys_addr_t fifo;
struct psc_dma *psc_dma;
@@ -487,8 +487,9 @@ out_unmap:
iounmap(regs);
return ret;
}
+EXPORT_SYMBOL_GPL(mpc5200_audio_dma_create);
-static int mpc5200_hpcd_remove(struct platform_device *op)
+int mpc5200_audio_dma_destroy(struct platform_device *op)
{
struct psc_dma *psc_dma = dev_get_drvdata(&op->dev);
@@ -510,24 +511,7 @@ static int mpc5200_hpcd_remove(struct platform_device *op)
return 0;
}
-
-static struct of_device_id mpc5200_hpcd_match[] = {
- { .compatible = "fsl,mpc5200-pcm", },
- {}
-};
-MODULE_DEVICE_TABLE(of, mpc5200_hpcd_match);
-
-static struct platform_driver mpc5200_hpcd_of_driver = {
- .probe = mpc5200_hpcd_probe,
- .remove = mpc5200_hpcd_remove,
- .driver = {
- .owner = THIS_MODULE,
- .name = "mpc5200-pcm-audio",
- .of_match_table = mpc5200_hpcd_match,
- }
-};
-
-module_platform_driver(mpc5200_hpcd_of_driver);
+EXPORT_SYMBOL_GPL(mpc5200_audio_dma_destroy);
MODULE_AUTHOR("Grant Likely <grant.likely@secretlab.ca>");
MODULE_DESCRIPTION("Freescale MPC5200 PSC in DMA mode ASoC Driver");
diff --git a/sound/soc/fsl/mpc5200_dma.h b/sound/soc/fsl/mpc5200_dma.h
index a3c0cd5..dff253f 100644
--- a/sound/soc/fsl/mpc5200_dma.h
+++ b/sound/soc/fsl/mpc5200_dma.h
@@ -81,4 +81,7 @@ to_psc_dma_stream(struct snd_pcm_substream *substream, struct psc_dma *psc_dma)
return &psc_dma->playback;
}
+int mpc5200_audio_dma_create(struct platform_device *op);
+int mpc5200_audio_dma_destroy(struct platform_device *op);
+
#endif /* __SOUND_SOC_FSL_MPC5200_DMA_H__ */
diff --git a/sound/soc/fsl/mpc5200_psc_ac97.c b/sound/soc/fsl/mpc5200_psc_ac97.c
index 2f70729..17b17b8 100644
--- a/sound/soc/fsl/mpc5200_psc_ac97.c
+++ b/sound/soc/fsl/mpc5200_psc_ac97.c
@@ -283,6 +283,10 @@ static int __devinit psc_ac97_of_probe(struct platform_device *op)
struct snd_ac97 ac97;
struct mpc52xx_psc __iomem *regs;
+ rc = mpc5200_audio_dma_create(op);
+ if (rc != 0)
+ return rc;
+
rc = snd_soc_register_dais(&op->dev, psc_ac97_dai, ARRAY_SIZE(psc_ac97_dai));
if (rc != 0) {
dev_err(&op->dev, "Failed to register DAI\n");
@@ -308,6 +312,7 @@ static int __devinit psc_ac97_of_probe(struct platform_device *op)
static int __devexit psc_ac97_of_remove(struct platform_device *op)
{
+ mpc5200_audio_dma_destroy(op);
snd_soc_unregister_dais(&op->dev, ARRAY_SIZE(psc_ac97_dai));
return 0;
}
diff --git a/sound/soc/fsl/mpc5200_psc_i2s.c b/sound/soc/fsl/mpc5200_psc_i2s.c
index 3cf21df..5710dc8 100644
--- a/sound/soc/fsl/mpc5200_psc_i2s.c
+++ b/sound/soc/fsl/mpc5200_psc_i2s.c
@@ -159,6 +159,10 @@ static int __devinit psc_i2s_of_probe(struct platform_device *op)
struct psc_dma *psc_dma;
struct mpc52xx_psc __iomem *regs;
+ rc = mpc5200_audio_dma_create(op);
+ if (rc != 0)
+ return rc;
+
rc = snd_soc_register_dais(&op->dev, psc_i2s_dai, ARRAY_SIZE(psc_i2s_dai));
if (rc != 0) {
pr_err("Failed to register DAI\n");
@@ -203,6 +207,7 @@ static int __devinit psc_i2s_of_probe(struct platform_device *op)
static int __devexit psc_i2s_of_remove(struct platform_device *op)
{
+ mpc5200_audio_dma_destroy(op);
snd_soc_unregister_dais(&op->dev, ARRAY_SIZE(psc_i2s_dai));
return 0;
}
--
1.7.2.5
^ permalink raw reply related
* Re: [PATCH -V9 09/11] arch/powerpc: Add 64TB support
From: Scott Wood @ 2012-09-12 0:58 UTC (permalink / raw)
To: Aneesh Kumar K.V; +Cc: paulus, linuxppc-dev
In-Reply-To: <1347281577-5460-10-git-send-email-aneesh.kumar@linux.vnet.ibm.com>
On 09/10/2012 07:52 AM, Aneesh Kumar K.V wrote:
> From: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
>
> Increase max addressable range to 64TB. This is not tested on
> real hardware yet.
>
> Reviewed-by: Paul Mackerras <paulus@samba.org>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
> ---
> arch/powerpc/include/asm/mmu-hash64.h | 42 ++++++++++++++++++++------
> arch/powerpc/include/asm/pgtable-ppc64-4k.h | 2 +-
> arch/powerpc/include/asm/pgtable-ppc64-64k.h | 2 +-
> arch/powerpc/include/asm/processor.h | 4 +--
> arch/powerpc/include/asm/sparsemem.h | 4 +--
> arch/powerpc/kernel/exceptions-64s.S | 4 ++-
> arch/powerpc/mm/slb_low.S | 12 ++++++++
> 7 files changed, 54 insertions(+), 16 deletions(-)
Bisect shows this patch to be the one that causes hangs on p5020. It
gets stuck in a loop of TLB misses, bailing on a PUD entry of zero. The
page fault path runs and returns, but doesn't fix the situation, so it
repeats. AFAICT the miss handler is behaving correctly. I'll try to
see what's going on in the page fault code.
-Scott
^ permalink raw reply
* Re: [PATCH 2/3] powerpc/esdhc: add property to disable the CMD23
From: Chris Ball @ 2012-09-11 20:59 UTC (permalink / raw)
To: Kumar Gala
Cc: linux-mmc, Huang Changming-R66093,
linuxppc-dev@lists.ozlabs.org list, Anton Vorontsov
In-Reply-To: <FC22FA36-B2CF-49C2-A0B1-8A0EA614B9CB@kernel.crashing.org>
Hi,
On Tue, Sep 11 2012, Kumar Gala wrote:
> thanks for the info. Do you know what's required on controller side
> to handle cards that support CMD23?
>
> I'm trying to figure out if older controller's on FSL SoCs are missing
> some feature to allow CMD23 to work (vs Auto-CMD23).
It seems plausible that it's just not implemented on these controllers.
It's a little strange, since the command's been specified for so long
and we haven't seen any other controllers with problems. The patch
would be correct if this is true.
- Chris.
--
Chris Ball <cjb@laptop.org> <http://printf.net/>
One Laptop Per Child
^ permalink raw reply
* Re: [PATCH 2/3] powerpc/esdhc: add property to disable the CMD23
From: Kumar Gala @ 2012-09-11 20:26 UTC (permalink / raw)
To: Chris Ball
Cc: linux-mmc, Huang Changming-R66093,
linuxppc-dev@lists.ozlabs.org list, Anton Vorontsov
In-Reply-To: <87k3w0y5yn.fsf@octavius.laptop.org>
On Sep 11, 2012, at 9:44 AM, Chris Ball wrote:
> Hi,
>=20
> On Tue, Sep 11 2012, Kumar Gala wrote:
>> In sdhci_add_host()
>>=20
>> We have the following
>>=20
>> ...
>> mmc->caps |=3D MMC_CAP_SDIO_IRQ | MMC_CAP_ERASE | =
MMC_CAP_CMD23;
>>=20
>> if (host->quirks & SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12)
>> host->flags |=3D SDHCI_AUTO_CMD12;
>>=20
>> /* Auto-CMD23 stuff only works in ADMA or PIO. */
>> if ((host->version >=3D SDHCI_SPEC_300) &&
>> ((host->flags & SDHCI_USE_ADMA) ||
>> !(host->flags & SDHCI_USE_SDMA))) {
>> host->flags |=3D SDHCI_AUTO_CMD23;
>> DBG("%s: Auto-CMD23 available\n", mmc_hostname(mmc));
>> } else {
>> DBG("%s: Auto-CMD23 unavailable\n", =
mmc_hostname(mmc));
>> }
>>=20
>> ...
>>=20
>> I'm not clear what the difference is between mmc->caps & host->flags, =
but shouldn't we move setting MMC_CAP_CMD23 inside the 'Auto-CMD23' if =
check?
>=20
> The main answer is: No, because CMD23 is distinct from Auto-CMD23.
>=20
> Multiblock transfers (CMD23) date back from MMC cards (which is why
> they're an MMC host capability) and can also be used in SDHCI.
>=20
> Auto-CMD23 is a new feature in SDHCI 3.0 that reduces the overhead
> of sending CMD23. It doesn't work if we're using SDMA, though.
>=20
> As for capability vs. flags, the capability is more of an inherent
> property of the controller, and flags are runtime decisions on whether
> to use that capability, based on e.g. the presence of a quirk.
>=20
> So, I think the code's correct as written. Feel free to ask more
> questions if you're investigating a specific problem that you haven't
> mentioned yet.
Chris,
thanks for the info. Do you know what's required on controller side to =
handle cards that support CMD23?
I'm trying to figure out if older controller's on FSL SoCs are missing =
some feature to allow CMD23 to work (vs Auto-CMD23).
- k=
^ permalink raw reply
* Re: [PATCH 2/3] powerpc/esdhc: add property to disable the CMD23
From: Scott Wood @ 2012-09-11 18:28 UTC (permalink / raw)
To: Anton Vorontsov; +Cc: Chang-Ming.Huang, linux-mmc, linuxppc-dev
In-Reply-To: <20120911080457.GA28235@lizard>
On 09/11/2012 03:04 AM, Anton Vorontsov wrote:
> On Tue, Sep 11, 2012 at 12:54:29AM -0700, Anton Vorontsov wrote:
>> On Tue, Sep 11, 2012 at 03:12:44PM +0800, Chang-Ming.Huang@freescale.com wrote:
>>> From: Jerry Huang <Chang-Ming.Huang@freescale.com>
>>>
>>> Below SOCs don't support the cmd23 command for MMC card,
>>> therefore, disable it in device tree:
>>> P1020, P1021, P1022, P1024, P1025 and P4080
>>>
>>> Signed-off-by: Jerry Huang <Chang-Ming.Huang@freescale.com>
>>
>> Acked-by: Anton Vorontsov <cbouatmailru@gmail.com>
>
> Btw, although the patch is trivial, I guess you still want to let know
> PowerPC folks about it. Adding Cc and copying the patch:
>
> - - - -
> From: Jerry Huang <Chang-Ming.Huang@freescale.com>
>
> Below SOCs don't support the cmd23 command for MMC card,
> therefore, disable it in device tree:
> P1020, P1021, P1022, P1024, P1025 and P4080
>
> Signed-off-by: Jerry Huang <Chang-Ming.Huang@freescale.com>
> CC: Anton Vorontsov <cbouatmailru@gmail.com>
> ---
> arch/powerpc/boot/dts/fsl/p1020si-post.dtsi | 1 +
> arch/powerpc/boot/dts/fsl/p1021si-post.dtsi | 1 +
> arch/powerpc/boot/dts/fsl/p1022si-post.dtsi | 1 +
> arch/powerpc/boot/dts/fsl/p4080si-post.dtsi | 1 +
> 4 files changed, 4 insertions(+)
>
> diff --git a/arch/powerpc/boot/dts/fsl/p1020si-post.dtsi b/arch/powerpc/boot/dts/fsl/p1020si-post.dtsi
> index 68cc5e7..793a30b 100644
> --- a/arch/powerpc/boot/dts/fsl/p1020si-post.dtsi
> +++ b/arch/powerpc/boot/dts/fsl/p1020si-post.dtsi
> @@ -154,6 +154,7 @@
> sdhc@2e000 {
> compatible = "fsl,p1020-esdhc", "fsl,esdhc";
> sdhci,auto-cmd12;
> + sdhci,no-cmd23;
> };
> /include/ "pq3-sec3.3-0.dtsi"
>
> diff --git a/arch/powerpc/boot/dts/fsl/p1021si-post.dtsi b/arch/powerpc/boot/dts/fsl/p1021si-post.dtsi
> index adb82fd..2b7fd2a 100644
> --- a/arch/powerpc/boot/dts/fsl/p1021si-post.dtsi
> +++ b/arch/powerpc/boot/dts/fsl/p1021si-post.dtsi
> @@ -149,6 +149,7 @@
> /include/ "pq3-esdhc-0.dtsi"
> sdhc@2e000 {
> sdhci,auto-cmd12;
> + sdhci,no-cmd23;
> };
>
> /include/ "pq3-sec3.3-0.dtsi"
> diff --git a/arch/powerpc/boot/dts/fsl/p1022si-post.dtsi b/arch/powerpc/boot/dts/fsl/p1022si-post.dtsi
> index 06216b8..2334a52 100644
> --- a/arch/powerpc/boot/dts/fsl/p1022si-post.dtsi
> +++ b/arch/powerpc/boot/dts/fsl/p1022si-post.dtsi
> @@ -215,6 +215,7 @@
> sdhc@2e000 {
> compatible = "fsl,p1022-esdhc", "fsl,esdhc";
> sdhci,auto-cmd12;
> + sdhci,no-cmd23;
> };
>
> /include/ "pq3-sec3.3-0.dtsi"
> diff --git a/arch/powerpc/boot/dts/fsl/p4080si-post.dtsi b/arch/powerpc/boot/dts/fsl/p4080si-post.dtsi
> index 8d35d2c..5b39952 100644
> --- a/arch/powerpc/boot/dts/fsl/p4080si-post.dtsi
> +++ b/arch/powerpc/boot/dts/fsl/p4080si-post.dtsi
> @@ -337,6 +337,7 @@
> sdhc@114000 {
> voltage-ranges = <3300 3300>;
> sdhci,auto-cmd12;
> + sdhci,no-cmd23;
> };
>
> /include/ "qoriq-i2c-0.dtsi"
>
This won't help people with old device trees (forked for a custom board,
tied to an old U-Boot, etc). The driver should infer this from the
compatible string or version registers (ideally block-specific version
registers, but if these are absent or inconclusive, SVR can be used).
-Scott
^ permalink raw reply
* Re: [PATCH 2/2] powerpc/e6500: TLB miss handler with hardware tablewalk support
From: Scott Wood @ 2012-09-11 17:24 UTC (permalink / raw)
To: Caraman Mihai Claudiu-B02008
Cc: Wood Scott-B07421, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <300B73AA675FCE4A93EB4FC1D42459FF1DC945@039-SN2MPN1-011.039d.mgd.msft.net>
On 09/11/2012 12:06 PM, Caraman Mihai Claudiu-B02008 wrote:
>> diff --git a/arch/powerpc/mm/tlb_low_64e.S
>> b/arch/powerpc/mm/tlb_low_64e.S
>> index efe0f33..8e82772 100644
>> --- a/arch/powerpc/mm/tlb_low_64e.S
>> +++ b/arch/powerpc/mm/tlb_low_64e.S
>> @@ -232,6 +232,173 @@ itlb_miss_fault_bolted:
>> beq tlb_miss_common_bolted
>> b itlb_miss_kernel_bolted
>>
>> +/*
>> + * TLB miss handling for e6500 and derivatives, using hardware
>> tablewalk.
>> + *
>> + * Linear mapping is bolted: no virtual page table or nested TLB misses
>> + * Indirect entries in TLB1, hardware loads resulting direct entries
>> + * into TLB0
>> + * No HES or NV hint on TLB1, so we need to do software round-robin
>> + * No tlbsrx. so we need a spinlock, and we have to deal
>> + * with MAS-damage caused by tlbsx
>> + * 4K pages only
>> + */
>> +
>> + START_EXCEPTION(instruction_tlb_miss_e6500)
>> + tlb_prolog_bolted SPRN_SRR0
>> +
>> + ld r11,PACA_TLB_PER_CORE_PTR(r13)
>> + srdi. r15,r16,60 /* get region */
>> + ori r16,r16,1
>> +
>> + TLB_MISS_STATS_SAVE_INFO_BOLTED
>> + bne tlb_miss_kernel_e6500 /* user/kernel test */
>> +
>> + b tlb_miss_common_e6500
>> +
>> + START_EXCEPTION(data_tlb_miss_e6500)
>> + tlb_prolog_bolted SPRN_DEAR
>> +
>> + ld r11,PACA_TLB_PER_CORE_PTR(r13)
>> + srdi. r15,r16,60 /* get region */
>> + rldicr r16,r16,0,62
>> +
>> + TLB_MISS_STATS_SAVE_INFO_BOLTED
>> + bne tlb_miss_kernel_e6500 /* user vs kernel check */
>> +
>
> This ends up calling DO_KVM macro twice with same parameters which
> generates the following compile error:
>
> arch/powerpc/mm/tlb_low_64e.S:307: Error: symbol `kvmppc_resume_14_0x01B' is already defined
> arch/powerpc/mm/tlb_low_64e.S:319: Error: symbol `kvmppc_resume_13_0x01B' is already defined
I assume the reason you don't already see this is because you only did
DO_KVM for the bolted version of the handlers.
> We can live with it if we patch DO_KVM like this:
>
> diff --git a/arch/powerpc/include/asm/kvm_booke_hv_asm.h b/arch/powerpc/include/asm/kvm_booke_hv_asm.h
> index 4610fb0..029ecab 100644
> --- a/arch/powerpc/include/asm/kvm_booke_hv_asm.h
> +++ b/arch/powerpc/include/asm/kvm_booke_hv_asm.h
> @@ -55,9 +55,9 @@
> #ifdef CONFIG_KVM_BOOKE_HV
> BEGIN_FTR_SECTION
> mtocrf 0x80, r11 /* check MSR[GS] without clobbering reg */
> - bf 3, kvmppc_resume_\intno\()_\srr1
> + bf 3, 1f
> b kvmppc_handler_\intno\()_\srr1
> -kvmppc_resume_\intno\()_\srr1:
> +1:
> END_FTR_SECTION_IFSET(CPU_FTR_EMB_HV)
> #endif
> .endm
Please do that, though maybe use a more unique label number in case the
calling context is using numbered labels.
-Scott
^ permalink raw reply
* RE: [PATCH 2/2] powerpc/e6500: TLB miss handler with hardware tablewalk support
From: Caraman Mihai Claudiu-B02008 @ 2012-09-11 17:06 UTC (permalink / raw)
To: Wood Scott-B07421, benh@kernel.crashing.org,
galak@kernel.crashing.org
Cc: linuxppc-dev@lists.ozlabs.org
In-Reply-To: <20120614234101.GB17147@tyr.buserror.net>
> diff --git a/arch/powerpc/mm/tlb_low_64e.S
> b/arch/powerpc/mm/tlb_low_64e.S
> index efe0f33..8e82772 100644
> --- a/arch/powerpc/mm/tlb_low_64e.S
> +++ b/arch/powerpc/mm/tlb_low_64e.S
> @@ -232,6 +232,173 @@ itlb_miss_fault_bolted:
> beq tlb_miss_common_bolted
> b itlb_miss_kernel_bolted
>=20
> +/*
> + * TLB miss handling for e6500 and derivatives, using hardware
> tablewalk.
> + *
> + * Linear mapping is bolted: no virtual page table or nested TLB misses
> + * Indirect entries in TLB1, hardware loads resulting direct entries
> + * into TLB0
> + * No HES or NV hint on TLB1, so we need to do software round-robin
> + * No tlbsrx. so we need a spinlock, and we have to deal
> + * with MAS-damage caused by tlbsx
> + * 4K pages only
> + */
> +
> + START_EXCEPTION(instruction_tlb_miss_e6500)
> + tlb_prolog_bolted SPRN_SRR0
> +
> + ld r11,PACA_TLB_PER_CORE_PTR(r13)
> + srdi. r15,r16,60 /* get region */
> + ori r16,r16,1
> +
> + TLB_MISS_STATS_SAVE_INFO_BOLTED
> + bne tlb_miss_kernel_e6500 /* user/kernel test */
> +
> + b tlb_miss_common_e6500
> +
> + START_EXCEPTION(data_tlb_miss_e6500)
> + tlb_prolog_bolted SPRN_DEAR
> +
> + ld r11,PACA_TLB_PER_CORE_PTR(r13)
> + srdi. r15,r16,60 /* get region */
> + rldicr r16,r16,0,62
> +
> + TLB_MISS_STATS_SAVE_INFO_BOLTED
> + bne tlb_miss_kernel_e6500 /* user vs kernel check */
> +
This ends up calling DO_KVM macro twice with same parameters which
generates the following compile error:
arch/powerpc/mm/tlb_low_64e.S:307: Error: symbol `kvmppc_resume_14_0x01B' =
is already defined
arch/powerpc/mm/tlb_low_64e.S:319: Error: symbol `kvmppc_resume_13_0x01B' =
is already defined
We can live with it if we patch DO_KVM like this:
diff --git a/arch/powerpc/include/asm/kvm_booke_hv_asm.h b/arch/powerpc/inc=
lude/asm/kvm_booke_hv_asm.h
index 4610fb0..029ecab 100644
--- a/arch/powerpc/include/asm/kvm_booke_hv_asm.h
+++ b/arch/powerpc/include/asm/kvm_booke_hv_asm.h
@@ -55,9 +55,9 @@
#ifdef CONFIG_KVM_BOOKE_HV
BEGIN_FTR_SECTION
mtocrf 0x80, r11 /* check MSR[GS] without clobbering reg */
- bf 3, kvmppc_resume_\intno\()_\srr1
+ bf 3, 1f
b kvmppc_handler_\intno\()_\srr1
-kvmppc_resume_\intno\()_\srr1:
+1:
END_FTR_SECTION_IFSET(CPU_FTR_EMB_HV)
#endif
.endm
-Mike
^ permalink raw reply related
* Re: [PATCH 2/3] powerpc/esdhc: add property to disable the CMD23
From: Chris Ball @ 2012-09-11 14:44 UTC (permalink / raw)
To: Kumar Gala
Cc: linux-mmc, Huang Changming-R66093,
linuxppc-dev@lists.ozlabs.org list, Anton Vorontsov
In-Reply-To: <2A6FCAE3-288F-4972-90C0-15105932F13E@kernel.crashing.org>
Hi,
On Tue, Sep 11 2012, Kumar Gala wrote:
> In sdhci_add_host()
>
> We have the following
>
> ...
> mmc->caps |= MMC_CAP_SDIO_IRQ | MMC_CAP_ERASE | MMC_CAP_CMD23;
>
> if (host->quirks & SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12)
> host->flags |= SDHCI_AUTO_CMD12;
>
> /* Auto-CMD23 stuff only works in ADMA or PIO. */
> if ((host->version >= SDHCI_SPEC_300) &&
> ((host->flags & SDHCI_USE_ADMA) ||
> !(host->flags & SDHCI_USE_SDMA))) {
> host->flags |= SDHCI_AUTO_CMD23;
> DBG("%s: Auto-CMD23 available\n", mmc_hostname(mmc));
> } else {
> DBG("%s: Auto-CMD23 unavailable\n", mmc_hostname(mmc));
> }
>
> ...
>
> I'm not clear what the difference is between mmc->caps & host->flags, but shouldn't we move setting MMC_CAP_CMD23 inside the 'Auto-CMD23' if check?
The main answer is: No, because CMD23 is distinct from Auto-CMD23.
Multiblock transfers (CMD23) date back from MMC cards (which is why
they're an MMC host capability) and can also be used in SDHCI.
Auto-CMD23 is a new feature in SDHCI 3.0 that reduces the overhead
of sending CMD23. It doesn't work if we're using SDMA, though.
As for capability vs. flags, the capability is more of an inherent
property of the controller, and flags are runtime decisions on whether
to use that capability, based on e.g. the presence of a quirk.
So, I think the code's correct as written. Feel free to ask more
questions if you're investigating a specific problem that you haven't
mentioned yet.
Thanks,
- Chris.
--
Chris Ball <cjb@laptop.org> <http://printf.net/>
One Laptop Per Child
^ permalink raw reply
* Re: linux-next: build failure after merge of the pci tree
From: Bjorn Helgaas @ 2012-09-11 14:28 UTC (permalink / raw)
To: Stephen Rothwell
Cc: Gavin Shan, linux-kernel, linux-next, Paul Mackerras, Yijing Wang,
linuxppc-dev
In-Reply-To: <20120911112924.c2206bc738a2c29259f6e2d8@canb.auug.org.au>
On Mon, Sep 10, 2012 at 7:29 PM, Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> Hi Bjorn,
>
> After merging the pci tree, today's linux-next build (powerpc
> ppc64_defconfig) failed like this:
>
> arch/powerpc/platforms/powernv/pci-ioda.c: In function 'pnv_pci_window_alignment':
> arch/powerpc/platforms/powernv/pci-ioda.c:1163:13: error: 'struct pci_dev' has no member named 'pcie_type'
>
> Caused by commit ac161fbfdaa2 ("powerpc/powernv: I/O and memory alignment
> for P2P bridges"). pcie_type was removed in commit b2ef39be5744 ("PCI:
> Remove unused field pcie_type from struct pci_dev") (also in the pci
> tree).
>
> I have used the pci tree from next-20120910 for today.
Thanks.
Gavin, Ben, I folded in the trivial fix and updated both my
pci/gavin-window-alignment and next branches.
Ben, I think you'll need to pull in pci/jiang-pcie-cap before
pci/gavin-window-alignment to get both pieces.
Bjorn
^ permalink raw reply
* Re: [PATCH 2/3] powerpc/esdhc: add property to disable the CMD23
From: Kumar Gala @ 2012-09-11 12:49 UTC (permalink / raw)
To: Huang Changming-R66093
Cc: linux-mmc, Chris Ball, linuxppc-dev@lists.ozlabs.org list,
Anton Vorontsov
In-Reply-To: <6D8C87BA-CA06-4215-94F5-63EBF5162B0A@kernel.crashing.org>
In sdhci_add_host()
We have the following
...
mmc->caps |=3D MMC_CAP_SDIO_IRQ | MMC_CAP_ERASE | MMC_CAP_CMD23;
if (host->quirks & SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12)
host->flags |=3D SDHCI_AUTO_CMD12;
/* Auto-CMD23 stuff only works in ADMA or PIO. */
if ((host->version >=3D SDHCI_SPEC_300) &&
((host->flags & SDHCI_USE_ADMA) ||
!(host->flags & SDHCI_USE_SDMA))) {
host->flags |=3D SDHCI_AUTO_CMD23;
DBG("%s: Auto-CMD23 available\n", mmc_hostname(mmc));
} else {
DBG("%s: Auto-CMD23 unavailable\n", mmc_hostname(mmc));
}
...
I'm not clear what the difference is between mmc->caps & host->flags, =
but shouldn't we move setting MMC_CAP_CMD23 inside the 'Auto-CMD23' if =
check?
- k=
^ permalink raw reply
* Re: [PATCH 2/3] powerpc/esdhc: add property to disable the CMD23
From: Kumar Gala @ 2012-09-11 12:43 UTC (permalink / raw)
To: Huang Changming-R66093
Cc: linuxppc-dev@lists.ozlabs.org, linux-mmc@vger.kernel.org,
Anton Vorontsov
In-Reply-To: <110EED8CC96DFC488B7E717A2027A27C176170@039-SN1MPN1-002.039d.mgd.msft.net>
On Sep 11, 2012, at 4:36 AM, Huang Changming-R66093 wrote:
> Thanks, Anton.
> If it is necessary, I will resend this patch to =
linuxppc-dev@lists.ozlabs.org.
>=20
> Best Regards
> Jerry Huang
I'm still not convinced this is the way to handle this issue. It seems =
as if the linux driver code makes some assumptions about CMD23 support =
that it shouldn't.
- k
>=20
>=20
>> -----Original Message-----
>> From: Anton Vorontsov [mailto:cbouatmailru@gmail.com]
>> Sent: Tuesday, September 11, 2012 4:05 PM
>> To: Huang Changming-R66093
>> Cc: linux-mmc@vger.kernel.org; Kumar Gala; =
linuxppc-dev@lists.ozlabs.org
>> Subject: Re: [PATCH 2/3] powerpc/esdhc: add property to disable the =
CMD23
>>=20
>> On Tue, Sep 11, 2012 at 12:54:29AM -0700, Anton Vorontsov wrote:
>>> On Tue, Sep 11, 2012 at 03:12:44PM +0800, Chang-
>> Ming.Huang@freescale.com wrote:
>>>> From: Jerry Huang <Chang-Ming.Huang@freescale.com>
>>>>=20
>>>> Below SOCs don't support the cmd23 command for MMC card, therefore,
>>>> disable it in device tree:
>>>> P1020, P1021, P1022, P1024, P1025 and P4080
>>>>=20
>>>> Signed-off-by: Jerry Huang <Chang-Ming.Huang@freescale.com>
>>>=20
>>> Acked-by: Anton Vorontsov <cbouatmailru@gmail.com>
>>=20
>> Btw, although the patch is trivial, I guess you still want to let =
know
>> PowerPC folks about it. Adding Cc and copying the patch:
>>=20
>> - - - -
>> From: Jerry Huang <Chang-Ming.Huang@freescale.com>
>>=20
>> Below SOCs don't support the cmd23 command for MMC card, therefore,
>> disable it in device tree:
>> P1020, P1021, P1022, P1024, P1025 and P4080
>>=20
>> Signed-off-by: Jerry Huang <Chang-Ming.Huang@freescale.com>
>> CC: Anton Vorontsov <cbouatmailru@gmail.com>
>> ---
>> arch/powerpc/boot/dts/fsl/p1020si-post.dtsi | 1 +
>> arch/powerpc/boot/dts/fsl/p1021si-post.dtsi | 1 +
>> arch/powerpc/boot/dts/fsl/p1022si-post.dtsi | 1 +
>> arch/powerpc/boot/dts/fsl/p4080si-post.dtsi | 1 +
>> 4 files changed, 4 insertions(+)
>>=20
>> diff --git a/arch/powerpc/boot/dts/fsl/p1020si-post.dtsi
>> b/arch/powerpc/boot/dts/fsl/p1020si-post.dtsi
>> index 68cc5e7..793a30b 100644
>> --- a/arch/powerpc/boot/dts/fsl/p1020si-post.dtsi
>> +++ b/arch/powerpc/boot/dts/fsl/p1020si-post.dtsi
>> @@ -154,6 +154,7 @@
>> sdhc@2e000 {
>> compatible =3D "fsl,p1020-esdhc", "fsl,esdhc";
>> sdhci,auto-cmd12;
>> + sdhci,no-cmd23;
>> };
>> /include/ "pq3-sec3.3-0.dtsi"
>>=20
>> diff --git a/arch/powerpc/boot/dts/fsl/p1021si-post.dtsi
>> b/arch/powerpc/boot/dts/fsl/p1021si-post.dtsi
>> index adb82fd..2b7fd2a 100644
>> --- a/arch/powerpc/boot/dts/fsl/p1021si-post.dtsi
>> +++ b/arch/powerpc/boot/dts/fsl/p1021si-post.dtsi
>> @@ -149,6 +149,7 @@
>> /include/ "pq3-esdhc-0.dtsi"
>> sdhc@2e000 {
>> sdhci,auto-cmd12;
>> + sdhci,no-cmd23;
>> };
>>=20
>> /include/ "pq3-sec3.3-0.dtsi"
>> diff --git a/arch/powerpc/boot/dts/fsl/p1022si-post.dtsi
>> b/arch/powerpc/boot/dts/fsl/p1022si-post.dtsi
>> index 06216b8..2334a52 100644
>> --- a/arch/powerpc/boot/dts/fsl/p1022si-post.dtsi
>> +++ b/arch/powerpc/boot/dts/fsl/p1022si-post.dtsi
>> @@ -215,6 +215,7 @@
>> sdhc@2e000 {
>> compatible =3D "fsl,p1022-esdhc", "fsl,esdhc";
>> sdhci,auto-cmd12;
>> + sdhci,no-cmd23;
>> };
>>=20
>> /include/ "pq3-sec3.3-0.dtsi"
>> diff --git a/arch/powerpc/boot/dts/fsl/p4080si-post.dtsi
>> b/arch/powerpc/boot/dts/fsl/p4080si-post.dtsi
>> index 8d35d2c..5b39952 100644
>> --- a/arch/powerpc/boot/dts/fsl/p4080si-post.dtsi
>> +++ b/arch/powerpc/boot/dts/fsl/p4080si-post.dtsi
>> @@ -337,6 +337,7 @@
>> sdhc@114000 {
>> voltage-ranges =3D <3300 3300>;
>> sdhci,auto-cmd12;
>> + sdhci,no-cmd23;
>> };
>>=20
>> /include/ "qoriq-i2c-0.dtsi"
>> --
>> 1.7.9.5
>=20
^ permalink raw reply
* [PATCH] Remove unused __get_user64() and __put_user64()
From: Bharat Bhushan @ 2012-09-11 10:19 UTC (permalink / raw)
To: linuxppc-dev, galak, benh; +Cc: Bharat Bhushan
__get_user64() and __put_user64() are not used.
Signed-off-by: Bharat Bhushan <bharat.bhushan@freescale.com>
---
arch/powerpc/include/asm/uaccess.h | 11 -----------
1 files changed, 0 insertions(+), 11 deletions(-)
diff --git a/arch/powerpc/include/asm/uaccess.h b/arch/powerpc/include/asm/uaccess.h
index 17bb40c..4db4959 100644
--- a/arch/powerpc/include/asm/uaccess.h
+++ b/arch/powerpc/include/asm/uaccess.h
@@ -98,11 +98,6 @@ struct exception_table_entry {
* PowerPC, we can just do these as direct assignments. (Of course, the
* exception handling means that it's no longer "just"...)
*
- * The "user64" versions of the user access functions are versions that
- * allow access of 64-bit data. The "get_user" functions do not
- * properly handle 64-bit data because the value gets down cast to a long.
- * The "put_user" functions already handle 64-bit data properly but we add
- * "user64" versions for completeness
*/
#define get_user(x, ptr) \
__get_user_check((x), (ptr), sizeof(*(ptr)))
@@ -114,12 +109,6 @@ struct exception_table_entry {
#define __put_user(x, ptr) \
__put_user_nocheck((__typeof__(*(ptr)))(x), (ptr), sizeof(*(ptr)))
-#ifndef __powerpc64__
-#define __get_user64(x, ptr) \
- __get_user64_nocheck((x), (ptr), sizeof(*(ptr)))
-#define __put_user64(x, ptr) __put_user(x, ptr)
-#endif
-
#define __get_user_inatomic(x, ptr) \
__get_user_nosleep((x), (ptr), sizeof(*(ptr)))
#define __put_user_inatomic(x, ptr) \
--
1.7.0.4
^ permalink raw reply related
* Re: [PATCH] i2c-mpc: Wait for STOP to hit the bus
From: Joakim Tjernlund @ 2012-09-11 10:13 UTC (permalink / raw)
Cc: linuxppc-dev@lists.ozlabs.org, Tabi Timur-B04825,
linux-i2c@vger.kernel.org
In-Reply-To: <OF0ACFB3FB.F2667AE9-ONC1257A6D.004E0ADE-C1257A6D.004EEB75@LocalDomain>
Joakim Tjernlund/Transmode wrote on 2012/09/02 16:22:00:
>
> Tabi Timur-B04825 <B04825@freescale.com> wrote on 2012/09/02 04:48:01:
> > On Thu, Aug 30, 2012 at 5:40 AM, Joakim Tjernlund
> > <Joakim.Tjernlund@transmode.se> wrote:
> >
> > > - mpc_i2c_stop(i2c);
> > > + mpc_i2c_stop(i2c); /* Initiate STOP */
> > > + orig_jiffies = jiffies;
> > > + /* Wait until STOP is seen, allow up to 1 s */
> > > + while (readb(i2c->base + MPC_I2C_SR) & CSR_MBB) {
> > > + if (time_after(jiffies, orig_jiffies + HZ)) {
> > > + u8 status = readb(i2c->base + MPC_I2C_SR);
> > > +
> > > + dev_dbg(i2c->dev, "timeout\n");
> > > + if ((status & (CSR_MCF | CSR_MBB | CSR_RXAK)) != 0) {
> > > + writeb(status & ~CSR_MAL,
> > > + i2c->base + MPC_I2C_SR);
> > > + mpc_i2c_fixup(i2c);
> > > + }
> > > + return -EIO;
> > > + }
> > > + cond_resched();
> > > + }
> >
> > Shouldn't the while-loop be inside mpc_i2c_stop() itself?
>
> Possibly but I choosed to do it this way as there is a similar loop in the beginning of mpc_xfer().
> I figured it has better visibility if it is in the same function.
>
> Jocke
Ping? Anything holding this patch back?
Jocke
^ permalink raw reply
* RE: [PATCH 2/3] powerpc/esdhc: add property to disable the CMD23
From: Huang Changming-R66093 @ 2012-09-11 9:36 UTC (permalink / raw)
To: Anton Vorontsov; +Cc: linuxppc-dev@lists.ozlabs.org, linux-mmc@vger.kernel.org
In-Reply-To: <20120911080457.GA28235@lizard>
VGhhbmtzLCBBbnRvbi4NCklmIGl0IGlzIG5lY2Vzc2FyeSwgSSB3aWxsIHJlc2VuZCB0aGlzIHBh
dGNoIHRvIGxpbnV4cHBjLWRldkBsaXN0cy5vemxhYnMub3JnLg0KDQpCZXN0IFJlZ2FyZHMNCkpl
cnJ5IEh1YW5nDQoNCg0KPiAtLS0tLU9yaWdpbmFsIE1lc3NhZ2UtLS0tLQ0KPiBGcm9tOiBBbnRv
biBWb3JvbnRzb3YgW21haWx0bzpjYm91YXRtYWlscnVAZ21haWwuY29tXQ0KPiBTZW50OiBUdWVz
ZGF5LCBTZXB0ZW1iZXIgMTEsIDIwMTIgNDowNSBQTQ0KPiBUbzogSHVhbmcgQ2hhbmdtaW5nLVI2
NjA5Mw0KPiBDYzogbGludXgtbW1jQHZnZXIua2VybmVsLm9yZzsgS3VtYXIgR2FsYTsgbGludXhw
cGMtZGV2QGxpc3RzLm96bGFicy5vcmcNCj4gU3ViamVjdDogUmU6IFtQQVRDSCAyLzNdIHBvd2Vy
cGMvZXNkaGM6IGFkZCBwcm9wZXJ0eSB0byBkaXNhYmxlIHRoZSBDTUQyMw0KPiANCj4gT24gVHVl
LCBTZXAgMTEsIDIwMTIgYXQgMTI6NTQ6MjlBTSAtMDcwMCwgQW50b24gVm9yb250c292IHdyb3Rl
Og0KPiA+IE9uIFR1ZSwgU2VwIDExLCAyMDEyIGF0IDAzOjEyOjQ0UE0gKzA4MDAsIENoYW5nLQ0K
PiBNaW5nLkh1YW5nQGZyZWVzY2FsZS5jb20gd3JvdGU6DQo+ID4gPiBGcm9tOiBKZXJyeSBIdWFu
ZyA8Q2hhbmctTWluZy5IdWFuZ0BmcmVlc2NhbGUuY29tPg0KPiA+ID4NCj4gPiA+IEJlbG93IFNP
Q3MgZG9uJ3Qgc3VwcG9ydCB0aGUgY21kMjMgY29tbWFuZCBmb3IgTU1DIGNhcmQsIHRoZXJlZm9y
ZSwNCj4gPiA+IGRpc2FibGUgaXQgaW4gZGV2aWNlIHRyZWU6DQo+ID4gPiBQMTAyMCwgUDEwMjEs
IFAxMDIyLCBQMTAyNCwgUDEwMjUgYW5kIFA0MDgwDQo+ID4gPg0KPiA+ID4gU2lnbmVkLW9mZi1i
eTogSmVycnkgSHVhbmcgPENoYW5nLU1pbmcuSHVhbmdAZnJlZXNjYWxlLmNvbT4NCj4gPg0KPiA+
IEFja2VkLWJ5OiBBbnRvbiBWb3JvbnRzb3YgPGNib3VhdG1haWxydUBnbWFpbC5jb20+DQo+IA0K
PiBCdHcsIGFsdGhvdWdoIHRoZSBwYXRjaCBpcyB0cml2aWFsLCBJIGd1ZXNzIHlvdSBzdGlsbCB3
YW50IHRvIGxldCBrbm93DQo+IFBvd2VyUEMgZm9sa3MgYWJvdXQgaXQuIEFkZGluZyBDYyBhbmQg
Y29weWluZyB0aGUgcGF0Y2g6DQo+IA0KPiAtIC0gLSAtDQo+IEZyb206IEplcnJ5IEh1YW5nIDxD
aGFuZy1NaW5nLkh1YW5nQGZyZWVzY2FsZS5jb20+DQo+IA0KPiBCZWxvdyBTT0NzIGRvbid0IHN1
cHBvcnQgdGhlIGNtZDIzIGNvbW1hbmQgZm9yIE1NQyBjYXJkLCB0aGVyZWZvcmUsDQo+IGRpc2Fi
bGUgaXQgaW4gZGV2aWNlIHRyZWU6DQo+IFAxMDIwLCBQMTAyMSwgUDEwMjIsIFAxMDI0LCBQMTAy
NSBhbmQgUDQwODANCj4gDQo+IFNpZ25lZC1vZmYtYnk6IEplcnJ5IEh1YW5nIDxDaGFuZy1NaW5n
Lkh1YW5nQGZyZWVzY2FsZS5jb20+DQo+IENDOiBBbnRvbiBWb3JvbnRzb3YgPGNib3VhdG1haWxy
dUBnbWFpbC5jb20+DQo+IC0tLQ0KPiAgYXJjaC9wb3dlcnBjL2Jvb3QvZHRzL2ZzbC9wMTAyMHNp
LXBvc3QuZHRzaSB8ICAgIDEgKw0KPiAgYXJjaC9wb3dlcnBjL2Jvb3QvZHRzL2ZzbC9wMTAyMXNp
LXBvc3QuZHRzaSB8ICAgIDEgKw0KPiAgYXJjaC9wb3dlcnBjL2Jvb3QvZHRzL2ZzbC9wMTAyMnNp
LXBvc3QuZHRzaSB8ICAgIDEgKw0KPiAgYXJjaC9wb3dlcnBjL2Jvb3QvZHRzL2ZzbC9wNDA4MHNp
LXBvc3QuZHRzaSB8ICAgIDEgKw0KPiAgNCBmaWxlcyBjaGFuZ2VkLCA0IGluc2VydGlvbnMoKykN
Cj4gDQo+IGRpZmYgLS1naXQgYS9hcmNoL3Bvd2VycGMvYm9vdC9kdHMvZnNsL3AxMDIwc2ktcG9z
dC5kdHNpDQo+IGIvYXJjaC9wb3dlcnBjL2Jvb3QvZHRzL2ZzbC9wMTAyMHNpLXBvc3QuZHRzaQ0K
PiBpbmRleCA2OGNjNWU3Li43OTNhMzBiIDEwMDY0NA0KPiAtLS0gYS9hcmNoL3Bvd2VycGMvYm9v
dC9kdHMvZnNsL3AxMDIwc2ktcG9zdC5kdHNpDQo+ICsrKyBiL2FyY2gvcG93ZXJwYy9ib290L2R0
cy9mc2wvcDEwMjBzaS1wb3N0LmR0c2kNCj4gQEAgLTE1NCw2ICsxNTQsNyBAQA0KPiAgCXNkaGNA
MmUwMDAgew0KPiAgCQljb21wYXRpYmxlID0gImZzbCxwMTAyMC1lc2RoYyIsICJmc2wsZXNkaGMi
Ow0KPiAgCQlzZGhjaSxhdXRvLWNtZDEyOw0KPiArCQlzZGhjaSxuby1jbWQyMzsNCj4gIAl9Ow0K
PiAgL2luY2x1ZGUvICJwcTMtc2VjMy4zLTAuZHRzaSINCj4gDQo+IGRpZmYgLS1naXQgYS9hcmNo
L3Bvd2VycGMvYm9vdC9kdHMvZnNsL3AxMDIxc2ktcG9zdC5kdHNpDQo+IGIvYXJjaC9wb3dlcnBj
L2Jvb3QvZHRzL2ZzbC9wMTAyMXNpLXBvc3QuZHRzaQ0KPiBpbmRleCBhZGI4MmZkLi4yYjdmZDJh
IDEwMDY0NA0KPiAtLS0gYS9hcmNoL3Bvd2VycGMvYm9vdC9kdHMvZnNsL3AxMDIxc2ktcG9zdC5k
dHNpDQo+ICsrKyBiL2FyY2gvcG93ZXJwYy9ib290L2R0cy9mc2wvcDEwMjFzaS1wb3N0LmR0c2kN
Cj4gQEAgLTE0OSw2ICsxNDksNyBAQA0KPiAgL2luY2x1ZGUvICJwcTMtZXNkaGMtMC5kdHNpIg0K
PiAgCXNkaGNAMmUwMDAgew0KPiAgCQlzZGhjaSxhdXRvLWNtZDEyOw0KPiArCQlzZGhjaSxuby1j
bWQyMzsNCj4gIAl9Ow0KPiANCj4gIC9pbmNsdWRlLyAicHEzLXNlYzMuMy0wLmR0c2kiDQo+IGRp
ZmYgLS1naXQgYS9hcmNoL3Bvd2VycGMvYm9vdC9kdHMvZnNsL3AxMDIyc2ktcG9zdC5kdHNpDQo+
IGIvYXJjaC9wb3dlcnBjL2Jvb3QvZHRzL2ZzbC9wMTAyMnNpLXBvc3QuZHRzaQ0KPiBpbmRleCAw
NjIxNmI4Li4yMzM0YTUyIDEwMDY0NA0KPiAtLS0gYS9hcmNoL3Bvd2VycGMvYm9vdC9kdHMvZnNs
L3AxMDIyc2ktcG9zdC5kdHNpDQo+ICsrKyBiL2FyY2gvcG93ZXJwYy9ib290L2R0cy9mc2wvcDEw
MjJzaS1wb3N0LmR0c2kNCj4gQEAgLTIxNSw2ICsyMTUsNyBAQA0KPiAgCXNkaGNAMmUwMDAgew0K
PiAgCQljb21wYXRpYmxlID0gImZzbCxwMTAyMi1lc2RoYyIsICJmc2wsZXNkaGMiOw0KPiAgCQlz
ZGhjaSxhdXRvLWNtZDEyOw0KPiArCQlzZGhjaSxuby1jbWQyMzsNCj4gIAl9Ow0KPiANCj4gIC9p
bmNsdWRlLyAicHEzLXNlYzMuMy0wLmR0c2kiDQo+IGRpZmYgLS1naXQgYS9hcmNoL3Bvd2VycGMv
Ym9vdC9kdHMvZnNsL3A0MDgwc2ktcG9zdC5kdHNpDQo+IGIvYXJjaC9wb3dlcnBjL2Jvb3QvZHRz
L2ZzbC9wNDA4MHNpLXBvc3QuZHRzaQ0KPiBpbmRleCA4ZDM1ZDJjLi41YjM5OTUyIDEwMDY0NA0K
PiAtLS0gYS9hcmNoL3Bvd2VycGMvYm9vdC9kdHMvZnNsL3A0MDgwc2ktcG9zdC5kdHNpDQo+ICsr
KyBiL2FyY2gvcG93ZXJwYy9ib290L2R0cy9mc2wvcDQwODBzaS1wb3N0LmR0c2kNCj4gQEAgLTMz
Nyw2ICszMzcsNyBAQA0KPiAgCXNkaGNAMTE0MDAwIHsNCj4gIAkJdm9sdGFnZS1yYW5nZXMgPSA8
MzMwMCAzMzAwPjsNCj4gIAkJc2RoY2ksYXV0by1jbWQxMjsNCj4gKwkJc2RoY2ksbm8tY21kMjM7
DQo+ICAJfTsNCj4gDQo+ICAvaW5jbHVkZS8gInFvcmlxLWkyYy0wLmR0c2kiDQo+IC0tDQo+IDEu
Ny45LjUNCg0K
^ permalink raw reply
* Re: [PATCH] vfio: enabled and supported on power (v7)
From: Alexey Kardashevskiy @ 2012-09-11 8:28 UTC (permalink / raw)
To: Alex Williamson; +Cc: Paul Mackerras, linuxppc-dev, David Gibson
In-Reply-To: <1347292924.24938.45.camel@ul30vt.home>
On 11/09/12 02:02, Alex Williamson wrote:
> On Tue, 2012-09-04 at 17:33 +1000, Alexey Kardashevskiy wrote:
>> Cc: David Gibson <david@gibson.dropbear.id.au>
>> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>> Cc: Paul Mackerras <paulus@samba.org>
>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>> ---
>
> Please at least cc kvm@vger as well since we list that as the devel list
> for vfio.
>
>> arch/powerpc/include/asm/iommu.h | 3 +
>
> I'll need an ack from Ben or Paul for this change.
>
>> drivers/iommu/Kconfig | 8 +
>> drivers/vfio/Kconfig | 6 +
>> drivers/vfio/Makefile | 1 +
>> drivers/vfio/vfio_iommu_spapr_tce.c | 440 +++++++++++++++++++++++++++++++++++
>> include/linux/vfio.h | 29 +++
>> 6 files changed, 487 insertions(+)
>> create mode 100644 drivers/vfio/vfio_iommu_spapr_tce.c
>>
>> diff --git a/arch/powerpc/include/asm/iommu.h b/arch/powerpc/include/asm/iommu.h
>> index 957a83f..c64bce7 100644
>> --- a/arch/powerpc/include/asm/iommu.h
>> +++ b/arch/powerpc/include/asm/iommu.h
>> @@ -66,6 +66,9 @@ struct iommu_table {
>> unsigned long it_halfpoint; /* Breaking point for small/large allocs */
>> spinlock_t it_lock; /* Protects it_map */
>> unsigned long *it_map; /* A simple allocation bitmap for now */
>> +#ifdef CONFIG_IOMMU_API
>> + struct iommu_group *it_group;
>> +#endif
>> };
>
> This seems to only be valid when vfio_iommu_spapr_tce is loaded, which
> is a bit misleading.
>
>>
>> struct scatterlist;
>> diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
>> index 3bd9fff..19cf2d9 100644
>> --- a/drivers/iommu/Kconfig
>> +++ b/drivers/iommu/Kconfig
>> @@ -162,4 +162,12 @@ config TEGRA_IOMMU_SMMU
>> space through the SMMU (System Memory Management Unit)
>> hardware included on Tegra SoCs.
>>
>> +config SPAPR_TCE_IOMMU
>> + bool "sPAPR TCE IOMMU Support"
>> + depends on PPC_PSERIES
>> + select IOMMU_API
>> + help
>> + Enables bits of IOMMU API required by VFIO. The iommu_ops is
>> + still not implemented.
>> +
>> endif # IOMMU_SUPPORT
>> diff --git a/drivers/vfio/Kconfig b/drivers/vfio/Kconfig
>> index 7cd5dec..b464687 100644
>> --- a/drivers/vfio/Kconfig
>> +++ b/drivers/vfio/Kconfig
>> @@ -3,10 +3,16 @@ config VFIO_IOMMU_TYPE1
>> depends on VFIO
>> default n
>>
>> +config VFIO_IOMMU_SPAPR_TCE
>> + tristate
>> + depends on VFIO && SPAPR_TCE_IOMMU
>> + default n
>> +
>> menuconfig VFIO
>> tristate "VFIO Non-Privileged userspace driver framework"
>> depends on IOMMU_API
>> select VFIO_IOMMU_TYPE1 if X86
>> + select VFIO_IOMMU_SPAPR_TCE if PPC_POWERNV
>> help
>> VFIO provides a framework for secure userspace device drivers.
>> See Documentation/vfio.txt for more details.
>> diff --git a/drivers/vfio/Makefile b/drivers/vfio/Makefile
>> index 2398d4a..72bfabc 100644
>> --- a/drivers/vfio/Makefile
>> +++ b/drivers/vfio/Makefile
>> @@ -1,3 +1,4 @@
>> obj-$(CONFIG_VFIO) += vfio.o
>> obj-$(CONFIG_VFIO_IOMMU_TYPE1) += vfio_iommu_type1.o
>> +obj-$(CONFIG_VFIO_IOMMU_SPAPR_TCE) += vfio_iommu_spapr_tce.o
>> obj-$(CONFIG_VFIO_PCI) += pci/
>> diff --git a/drivers/vfio/vfio_iommu_spapr_tce.c b/drivers/vfio/vfio_iommu_spapr_tce.c
>> new file mode 100644
>> index 0000000..21f1909
>> --- /dev/null
>> +++ b/drivers/vfio/vfio_iommu_spapr_tce.c
>> @@ -0,0 +1,440 @@
>> +/*
>> + * VFIO: IOMMU DMA mapping support for TCE on POWER
>> + *
>> + * Copyright (C) 2012 IBM Corp. All rights reserved.
>> + * Author: Alexey Kardashevskiy <aik@ozlabs.ru>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + *
>> + * Derived from original vfio_iommu_x86.c:
>
> Should this be _type1? Only the mail archives are going to remember
> there was a _x86, so the renamed version is probably a better reference.
>
>> + * Copyright (C) 2012 Red Hat, Inc. All rights reserved.
>> + * Author: Alex Williamson <alex.williamson@redhat.com>
>> + */
>> +
>> +#include <linux/module.h>
>> +#include <linux/pci.h>
>> +#include <linux/slab.h>
>> +#include <linux/uaccess.h>
>> +#include <linux/err.h>
>> +#include <linux/vfio.h>
>> +#include <linux/spinlock.h>
>> +#include <asm/iommu.h>
>> +
>> +#define DRIVER_VERSION "0.1"
>> +#define DRIVER_AUTHOR "aik@ozlabs.ru"
>> +#define DRIVER_DESC "VFIO IOMMU SPAPR TCE"
>> +
>> +
>> +/*
>> + * SPAPR TCE API
>> + */
>> +static void tce_free(struct iommu_table *tbl, unsigned long entry,
>> + unsigned long tce)
>> +{
>> + struct page *page = pfn_to_page(tce >> PAGE_SHIFT);
>> +
>> + WARN_ON(!page);
>> + if (page) {
>> + if (tce & VFIO_SPAPR_TCE_WRITE)
>> + SetPageDirty(page);
>> + put_page(page);
>> + }
>> + ppc_md.tce_free(tbl, entry, 1);
>> +}
>> +
>> +static long tce_put(struct iommu_table *tbl,
>> + unsigned long entry, uint64_t tce, uint32_t flags)
>> +{
>> + int ret;
>> + unsigned long oldtce, kva, offset;
>> + struct page *page = NULL;
>> + enum dma_data_direction direction = DMA_NONE;
>> +
>> + switch (flags & VFIO_SPAPR_TCE_PUT_MASK) {
>> + case VFIO_SPAPR_TCE_READ:
>> + direction = DMA_TO_DEVICE;
>> + break;
>> + case VFIO_SPAPR_TCE_WRITE:
>> + direction = DMA_FROM_DEVICE;
>> + break;
>> + case VFIO_SPAPR_TCE_BIDIRECTIONAL:
>> + direction = DMA_BIDIRECTIONAL;
>> + break;
>> + }
>> +
>> + oldtce = ppc_md.tce_get(tbl, entry);
>> +
>> + /* Free page if still allocated */
>> + if (oldtce & VFIO_SPAPR_TCE_PUT_MASK)
>> + tce_free(tbl, entry, oldtce);
>> +
>> + /* Map new TCE */
>> + if (direction != DMA_NONE) {
>> + offset = (tce & IOMMU_PAGE_MASK) - (tce & PAGE_MASK);
>> + ret = get_user_pages_fast(tce & PAGE_MASK, 1,
>> + direction != DMA_TO_DEVICE, &page);
>> + BUG_ON(ret > 1);
>
> Can this happen?
>
>> + if (ret < 1) {
>> + printk(KERN_ERR "tce_vfio: get_user_pages_fast failed "
>> + "tce=%llx ioba=%lx ret=%d\n",
>> + tce, entry << IOMMU_PAGE_SHIFT, ret);
>> + if (!ret)
>> + ret = -EFAULT;
>> + goto unlock_exit;
>> + }
>> +
>> + kva = (unsigned long) page_address(page);
>> + kva += offset;
>> + BUG_ON(!kva);
>
> Same here, can it happen? If so, should it BUG or catch the below
> EINVAL?
>
>> + if (WARN_ON(kva & ~IOMMU_PAGE_MASK))
>> + return -EINVAL;
>
> Page leak? Don't we want to do a put_page(), which means we probably
> want a goto exit here.
>
>> +
>> + /* Preserve access bits */
>> + kva |= flags & VFIO_SPAPR_TCE_PUT_MASK;
>> +
>> + /* tce_build receives a virtual address */
>> + entry += tbl->it_offset; /* Offset into real TCE table */
>> + ret = ppc_md.tce_build(tbl, entry, 1, kva, direction, NULL);
>> +
>> + /* tce_build() only returns non-zero for transient errors */
>> + if (unlikely(ret)) {
>> + printk(KERN_ERR "tce_vfio: Failed to add TCE\n");
>> + ret = -EIO;
>> + goto unlock_exit;
>> + }
>> + }
>> + /* Flush/invalidate TLB caches if necessary */
>> + if (ppc_md.tce_flush)
>> + ppc_md.tce_flush(tbl);
>> +
>> + /* Make sure updates are seen by hardware */
>> + mb();
>> +
>> +unlock_exit:
>
> unlock seems wrong here, I had to go re-read the code looking for the
> lock.
>
>> + if (ret && page)
>> + put_page(page);
>> +
>> + if (ret)
>> + printk(KERN_ERR "tce_vfio: tce_put failed on tce=%llx "
>> + "ioba=%lx kva=%lx\n", tce,
>> + entry << IOMMU_PAGE_SHIFT, kva);
>> + return ret;
>> +}
>> +
>> +/*
>> + * VFIO IOMMU fd for SPAPR_TCE IOMMU implementation
>> + */
>> +
>> +/*
>> + * The container descriptor supports only a single group per container.
>> + * Required by the API as the container is not supplied with the IOMMU group
>> + * at the moment of initialization.
>> + */
>> +struct tce_container {
>> + struct iommu_table *tbl;
>> +};
>> +
>> +static void *tce_iommu_open(unsigned long arg)
>> +{
>> + struct tce_container *container;
>> +
>> + if (arg != VFIO_SPAPR_TCE_IOMMU) {
>> + printk(KERN_ERR "tce_vfio: Wrong IOMMU type\n");
>> + return ERR_PTR(-EINVAL);
>> + }
>> +
>> + container = kzalloc(sizeof(*container), GFP_KERNEL);
>> + if (!container)
>> + return ERR_PTR(-ENOMEM);
>> +
>> + return container;
>> +}
>> +
>> +static void tce_iommu_release(void *iommu_data)
>> +{
>> + struct tce_container *container = iommu_data;
>> + struct iommu_table *tbl = container->tbl;
>> + unsigned long i, tce;
>> +
>
> This will segfault if releasing a container that never had an a device
> attached.
>
>> + /* Unmap leftovers */
>> + spin_lock_irq(&tbl->it_lock);
>> + for (i = tbl->it_offset; i < tbl->it_offset + tbl->it_size; ++i) {
>> + tce = ppc_md.tce_get(tbl, i);
>> + if (tce & VFIO_SPAPR_TCE_PUT_MASK)
>> + tce_free(tbl, i, tce);
>> + }
>> + /* Flush/invalidate TLB caches if necessary */
>> + if (ppc_md.tce_flush)
>> + ppc_md.tce_flush(tbl);
>> +
>> + /* Make sure updates are seen by hardware */
>> + mb();
>> +
>> + spin_unlock_irq(&tbl->it_lock);
>> +
>> + kfree(container);
>> +}
>> +
>> +static long tce_iommu_ioctl(void *iommu_data,
>> + unsigned int cmd, unsigned long arg)
>> +{
>> + struct tce_container *container = iommu_data;
>> + unsigned long minsz;
>> + long ret;
>> +
>> + switch (cmd) {
>> + case VFIO_CHECK_EXTENSION: {
>> + return (arg == VFIO_SPAPR_TCE_IOMMU) ? 1 : 0;
>> + }
>> + case VFIO_IOMMU_SPAPR_TCE_GET_INFO: {
>> + struct vfio_iommu_spapr_tce_info info;
>> + struct iommu_table *tbl = container->tbl;
>> +
>> + minsz = offsetofend(struct vfio_iommu_spapr_tce_info,
>> + dma64_window_size);
>> +
>> + if (copy_from_user(&info, (void __user *)arg, minsz))
>> + return -EFAULT;
>> +
>> + if (info.argsz < minsz)
>> + return -EINVAL;
>> +
>> + if (!tbl)
>> + return -ENXIO;
>
> nit: why not check this earlier?
>
>> +
>> + info.dma32_window_start = tbl->it_offset << IOMMU_PAGE_SHIFT;
>> + info.dma32_window_size = tbl->it_size << IOMMU_PAGE_SHIFT;
>> + info.dma64_window_start = 0;
>> + info.dma64_window_size = 0;
>> + info.flags = 0;
>> +
>> + return copy_to_user((void __user *)arg, &info, minsz);
>> + }
>> + case VFIO_IOMMU_SPAPR_TCE_PUT: {
>> + struct vfio_iommu_spapr_tce_put par;
>> + struct iommu_table *tbl = container->tbl;
>> +
>> + minsz = offsetofend(struct vfio_iommu_spapr_tce_put, tce);
>> +
>> + if (copy_from_user(&par, (void __user *)arg, minsz))
>> + return -EFAULT;
>> +
>> + if (par.argsz < minsz)
>> + return -EINVAL;
>> +
>> + if (!tbl) {
>> + return -ENXIO;
>> + }
>
> Same, plus drop the braces.
>
>> +
>> + spin_lock_irq(&tbl->it_lock);
>> + ret = tce_put(tbl, par.ioba >> IOMMU_PAGE_SHIFT,
>> + par.tce, par.flags);
>> + spin_unlock_irq(&tbl->it_lock);
>> +
>> + return ret;
>> + }
>
> Is "PUT" really the name we want for this?
Yes, it is a single H_PUT_TCE hypercall from POWER architecture spec.
>> + default:
>> + printk(KERN_WARNING "tce_vfio: unexpected cmd %x\n", cmd);
>> + }
>> +
>> + return -ENOTTY;
>> +}
>> +
>> +static int tce_iommu_attach_group(void *iommu_data,
>> + struct iommu_group *iommu_group)
>> +{
>> + struct tce_container *container = iommu_data;
>> + struct iommu_table *tbl = iommu_group_get_iommudata(iommu_group);
>> +
>> + printk(KERN_DEBUG "tce_vfio: Attaching group #%u to iommu %p\n",
>> + iommu_group_id(iommu_group), iommu_group);
>
> Let's use pr_debug() and friends throughout.
>
>> + if (container->tbl) {
>> + printk(KERN_WARNING "tce_vfio: Only one group per IOMMU "
>> + "container is allowed, "
>> + "existing id=%d, attaching id=%d\n",
>> + iommu_group_id(container->tbl->it_group),
>> + iommu_group_id(iommu_group));
>> + return -EBUSY;
>> + }
>> +
>
> _type1 has a lock to avoid races here, I think you might need one too.
>
>> + container->tbl = tbl;
>> +
>> + return 0;
>> +}
>> +
>> +static void tce_iommu_detach_group(void *iommu_data,
>> + struct iommu_group *iommu_group)
>> +{
>> + struct tce_container *container = iommu_data;
>> + struct iommu_table *tbl = iommu_group_get_iommudata(iommu_group);
>> +
>> + BUG_ON(!tbl);
>
> Needed? If so, why is there no check on attach?
Added to attach() :)
>
>> + if (tbl != container->tbl) {
>> + printk(KERN_WARNING "tce_vfio: detaching group #%u, expected "
>> + "group is #%u\n", iommu_group_id(iommu_group),
>> + iommu_group_id(tbl->it_group));
>> + return;
>> + }
>> + printk(KERN_DEBUG "tce_vfio: detaching group #%u from iommu %p\n",
>> + iommu_group_id(iommu_group), iommu_group);
>
> container->tbl = NULL?
Then I won't be able to release pages in tce_iommu_release().
Releasing pages in tce_iommu_detach_group() caused some other problems,
cannot recall now which ones.
>> +}
>> +
>> +const struct vfio_iommu_driver_ops tce_iommu_driver_ops = {
>> + .name = "iommu-vfio-powerpc",
>> + .owner = THIS_MODULE,
>> + .open = tce_iommu_open,
>> + .release = tce_iommu_release,
>> + .ioctl = tce_iommu_ioctl,
>> + .attach_group = tce_iommu_attach_group,
>> + .detach_group = tce_iommu_detach_group,
>> +};
>> +
>> +/*
>> + * Add/delete devices support (hotplug, module_init, module_exit)
>> + */
>> +static int add_device(struct device *dev)
>> +{
>> + struct iommu_table *tbl;
>> + int ret = 0;
>> +
>> + if (dev->iommu_group) {
>> + printk(KERN_WARNING "tce_vfio: device %s is already in iommu "
>> + "group %d, skipping\n", dev->kobj.name,
>
> Watch line wrapping on strings.
Pardon?
>> + iommu_group_id(dev->iommu_group));
>> + return -EBUSY;
>> + }
>> +
>> + tbl = get_iommu_table_base(dev);
>> + if (!tbl) {
>> + printk(KERN_DEBUG "tce_vfio: skipping device %s with no tbl\n",
>> + dev->kobj.name);
>> + return 0;
>> + }
>> +
>> + printk(KERN_DEBUG "tce_vfio: adding %s to iommu group %d\n",
>> + dev->kobj.name, iommu_group_id(tbl->it_group));
>> +
>> + ret = iommu_group_add_device(tbl->it_group, dev);
>> + if (ret < 0)
>> + printk(KERN_ERR "tce_vfio: %s has not been added, ret=%d\n",
>> + dev->kobj.name, ret);
>> +
>> + return ret;
>> +}
>> +
>> +static void 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 add_device(dev);
>> + case BUS_NOTIFY_DEL_DEVICE:
>> + del_device(dev);
>> + return 0;
>> + default:
>> + return 0;
>> + }
>> +}
>> +
>> +static struct notifier_block tce_iommu_bus_nb = {
>> + .notifier_call = iommu_bus_notifier,
>> +};
>> +
>> +void group_release(void *iommu_data)
>> +{
>> + struct iommu_table *tbl = iommu_data;
>> + tbl->it_group = NULL;
>> +}
>> +
>> +static int __init tce_iommu_init(void)
>> +{
>> + struct pci_dev *pdev = NULL;
>> + struct iommu_table *tbl;
>> + struct iommu_group *grp;
>> +
>> + /* If the current platform does not support tce_get
>> + we are unable to clean TCE table properly and
>> + therefore it is better not to touch it at all */
>> + if (!ppc_md.tce_get) {
>> + printk(KERN_ERR "tce_vfio: ppc_md.tce_get isn't implemented\n");
>> + return -EOPNOTSUPP;
>> + }
>> +
>> + bus_register_notifier(&pci_bus_type, &tce_iommu_bus_nb);
>> +
>> + /* Allocate and initialize VFIO groups */
>
> s/VFIO groups/IOMMU groups/
>
>> + for_each_pci_dev(pdev) {
>> + tbl = get_iommu_table_base(&pdev->dev);
>> + if (!tbl)
>> + continue;
>> +
>> + /* Skip already initialized */
>> + if (tbl->it_group)
>> + continue;
>> +
>> + grp = iommu_group_alloc();
>> + if (IS_ERR(grp)) {
>> + printk(KERN_INFO "tce_vfio: cannot create "
>> + "new IOMMU group, ret=%ld\n",
>> + PTR_ERR(grp));
>> + return -EFAULT;
>> + }
>> + tbl->it_group = grp;
>> + iommu_group_set_iommudata(grp, tbl, group_release);
>> + }
>> +
>> + /* Add PCI devices to VFIO groups */
>> + for_each_pci_dev(pdev)
>> + add_device(&pdev->dev);
>> +
>> + return vfio_register_iommu_driver(&tce_iommu_driver_ops);
>> +}
>> +
>> +static void __exit tce_iommu_cleanup(void)
>> +{
>> + struct pci_dev *pdev = NULL;
>> + struct iommu_table *tbl;
>> + struct iommu_group *grp = NULL;
>> +
>> + bus_unregister_notifier(&pci_bus_type, &tce_iommu_bus_nb);
>> +
>> + /* Delete PCI devices from VFIO groups */
>> + for_each_pci_dev(pdev)
>> + del_device(&pdev->dev);
>> +
>> + /* Release VFIO groups */
>> + for_each_pci_dev(pdev) {
>> + tbl = get_iommu_table_base(&pdev->dev);
>> + if (!tbl)
>> + continue;
>> + grp = tbl->it_group;
>> +
>> + /* Skip (already) uninitialized */
>> + if (!grp)
>> + continue;
>> +
>> + /* Do actual release, group_release() is expected to work */
>> + iommu_group_put(grp);
>> + BUG_ON(tbl->it_group);
>> + }
>> +
>
>
> It troubles me a bit that you're using the vfio driver to initialize and
> tear down IOMMU groups on your platform.
I am not following you here. Could you please explain a bit?
> VFIO makes use of IOMMU groups
> and is the only user so far, but they're hopefully useful beyond this.
> In fact, VFIO used to manage assembling all groups from data provided by
> the IOMMU but David wanted to see IOMMU groups be a more universally
> available feature, so it's odd to see POWER implementing it this way.
David, help! :)
>> + vfio_unregister_iommu_driver(&tce_iommu_driver_ops);
>> +}
>> +
>> +module_init(tce_iommu_init);
>> +module_exit(tce_iommu_cleanup);
>> +
>> +MODULE_VERSION(DRIVER_VERSION);
>> +MODULE_LICENSE("GPL v2");
>> +MODULE_AUTHOR(DRIVER_AUTHOR);
>> +MODULE_DESCRIPTION(DRIVER_DESC);
>> +
>> diff --git a/include/linux/vfio.h b/include/linux/vfio.h
>> index 0a4f180..2c0a927 100644
>> --- a/include/linux/vfio.h
>> +++ b/include/linux/vfio.h
>> @@ -99,6 +99,7 @@ extern void vfio_unregister_iommu_driver(
>> /* Extensions */
>>
>> #define VFIO_TYPE1_IOMMU 1
>> +#define VFIO_SPAPR_TCE_IOMMU 2
>>
>> /*
>> * The IOCTL interface is designed for extensibility by embedding the
>> @@ -442,4 +443,32 @@ struct vfio_iommu_type1_dma_unmap {
>>
>> #define VFIO_IOMMU_UNMAP_DMA _IO(VFIO_TYPE, VFIO_BASE + 14)
>>
>> +/* -------- API for SPAPR TCE (Server POWERPC) IOMMU -------- */
>> +
>> +struct vfio_iommu_spapr_tce_info {
>> + __u32 argsz;
>> + __u32 flags;
>> + __u32 dma32_window_start;
>> + __u32 dma32_window_size;
>> + __u64 dma64_window_start;
>> + __u64 dma64_window_size;
>> +};
>> +
>> +#define VFIO_IOMMU_SPAPR_TCE_GET_INFO _IO(VFIO_TYPE, VFIO_BASE + 12)
>> +
>> +struct vfio_iommu_spapr_tce_put {
>> + __u32 argsz;
>> + __u32 flags;
>> +#define VFIO_SPAPR_TCE_READ 1
>> +#define VFIO_SPAPR_TCE_WRITE 2
>> +#define VFIO_SPAPR_TCE_BIDIRECTIONAL (VFIO_SPAPR_TCE_READ|VFIO_SPAPR_TCE_WRITE)
>> +#define VFIO_SPAPR_TCE_PUT_MASK VFIO_SPAPR_TCE_BIDIRECTIONAL
>> + __u64 ioba;
>> + __u64 tce;
>> +};
>
> Ok, so if READ & WRITE are both clear and ioba is set, that's an
> "unmap"? This is exactly why _type1 has a MAP and UNMAP, to make it
> clear which fields are necessary for which call. I think we should
> probably do the same here. Besides, _put makes me think there should be
> a _get; do these have some unique meaning in POWER?
It is a single H_PUT_TCE for putting a record into TCE table. The guest
calls H_PUT_TCE, QEMU replaces the address and simply forwards the call to
the host. Calling them map/unmap makes it less clear for powerpc people :)
>
>> +
>> +#define VFIO_IOMMU_SPAPR_TCE_PUT _IO(VFIO_TYPE, VFIO_BASE + 13)
>> +
>
> Please document what all of the above means. Thanks,
Something like this?
/*
* The VFIO_IOMMU_SPAPR_TCE_PUT is implemented as the H_PUT_TCE hypercall.
* ioba - I/O Bus Address for indexing into TCE table
* tce - logical address of storage
*
* The non-zero flags means adding new page into the table.
* The zero flags means releasing the existing page and clearing the
* TCE table entry.
*/
> Alex
>
>> +/* ***************************************************************** */
>> +
>> #endif /* VFIO_H */
>
>
>
--
Alexey
^ permalink raw reply
* Re: [PATCH 2/3] powerpc/esdhc: add property to disable the CMD23
From: Anton Vorontsov @ 2012-09-11 8:04 UTC (permalink / raw)
To: Chang-Ming.Huang; +Cc: linuxppc-dev, linux-mmc
In-Reply-To: <20120911075429.GA27028@lizard>
On Tue, Sep 11, 2012 at 12:54:29AM -0700, Anton Vorontsov wrote:
> On Tue, Sep 11, 2012 at 03:12:44PM +0800, Chang-Ming.Huang@freescale.com wrote:
> > From: Jerry Huang <Chang-Ming.Huang@freescale.com>
> >
> > Below SOCs don't support the cmd23 command for MMC card,
> > therefore, disable it in device tree:
> > P1020, P1021, P1022, P1024, P1025 and P4080
> >
> > Signed-off-by: Jerry Huang <Chang-Ming.Huang@freescale.com>
>
> Acked-by: Anton Vorontsov <cbouatmailru@gmail.com>
Btw, although the patch is trivial, I guess you still want to let know
PowerPC folks about it. Adding Cc and copying the patch:
- - - -
From: Jerry Huang <Chang-Ming.Huang@freescale.com>
Below SOCs don't support the cmd23 command for MMC card,
therefore, disable it in device tree:
P1020, P1021, P1022, P1024, P1025 and P4080
Signed-off-by: Jerry Huang <Chang-Ming.Huang@freescale.com>
CC: Anton Vorontsov <cbouatmailru@gmail.com>
---
arch/powerpc/boot/dts/fsl/p1020si-post.dtsi | 1 +
arch/powerpc/boot/dts/fsl/p1021si-post.dtsi | 1 +
arch/powerpc/boot/dts/fsl/p1022si-post.dtsi | 1 +
arch/powerpc/boot/dts/fsl/p4080si-post.dtsi | 1 +
4 files changed, 4 insertions(+)
diff --git a/arch/powerpc/boot/dts/fsl/p1020si-post.dtsi b/arch/powerpc/boot/dts/fsl/p1020si-post.dtsi
index 68cc5e7..793a30b 100644
--- a/arch/powerpc/boot/dts/fsl/p1020si-post.dtsi
+++ b/arch/powerpc/boot/dts/fsl/p1020si-post.dtsi
@@ -154,6 +154,7 @@
sdhc@2e000 {
compatible = "fsl,p1020-esdhc", "fsl,esdhc";
sdhci,auto-cmd12;
+ sdhci,no-cmd23;
};
/include/ "pq3-sec3.3-0.dtsi"
diff --git a/arch/powerpc/boot/dts/fsl/p1021si-post.dtsi b/arch/powerpc/boot/dts/fsl/p1021si-post.dtsi
index adb82fd..2b7fd2a 100644
--- a/arch/powerpc/boot/dts/fsl/p1021si-post.dtsi
+++ b/arch/powerpc/boot/dts/fsl/p1021si-post.dtsi
@@ -149,6 +149,7 @@
/include/ "pq3-esdhc-0.dtsi"
sdhc@2e000 {
sdhci,auto-cmd12;
+ sdhci,no-cmd23;
};
/include/ "pq3-sec3.3-0.dtsi"
diff --git a/arch/powerpc/boot/dts/fsl/p1022si-post.dtsi b/arch/powerpc/boot/dts/fsl/p1022si-post.dtsi
index 06216b8..2334a52 100644
--- a/arch/powerpc/boot/dts/fsl/p1022si-post.dtsi
+++ b/arch/powerpc/boot/dts/fsl/p1022si-post.dtsi
@@ -215,6 +215,7 @@
sdhc@2e000 {
compatible = "fsl,p1022-esdhc", "fsl,esdhc";
sdhci,auto-cmd12;
+ sdhci,no-cmd23;
};
/include/ "pq3-sec3.3-0.dtsi"
diff --git a/arch/powerpc/boot/dts/fsl/p4080si-post.dtsi b/arch/powerpc/boot/dts/fsl/p4080si-post.dtsi
index 8d35d2c..5b39952 100644
--- a/arch/powerpc/boot/dts/fsl/p4080si-post.dtsi
+++ b/arch/powerpc/boot/dts/fsl/p4080si-post.dtsi
@@ -337,6 +337,7 @@
sdhc@114000 {
voltage-ranges = <3300 3300>;
sdhci,auto-cmd12;
+ sdhci,no-cmd23;
};
/include/ "qoriq-i2c-0.dtsi"
--
1.7.9.5
^ permalink raw reply related
* Re: [PATCH 13/25] macintosh/mediabay: add a const qualifier
From: Benjamin Herrenschmidt @ 2012-09-11 7:08 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Arnd Bergmann, Rob Herring, kernel, linuxppc-dev,
linux-arm-kernel
In-Reply-To: <20120911065604.GE28643@pengutronix.de>
On Tue, 2012-09-11 at 08:56 +0200, Uwe Kleine-König wrote:
>
> in the hope I didn't interpret "anything powerpc" too wide. Please
> tell me if I did.
No, it's fine with me, give me a git URL if you want me to run that
through my various config test builds.
Cheers,
Ben.
^ permalink raw reply
* Re: [PATCH 13/25] macintosh/mediabay: add a const qualifier
From: Uwe Kleine-König @ 2012-09-11 6:56 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: Arnd Bergmann, Rob Herring, kernel, linuxppc-dev,
linux-arm-kernel
In-Reply-To: <1346837023.2257.57.camel@pasglop>
On Wed, Sep 05, 2012 at 07:23:43PM +1000, Benjamin Herrenschmidt wrote:
> On Wed, 2012-09-05 at 10:02 +0200, Uwe Kleine-König wrote:
> > Hello,
> >
> > On Wed, Sep 05, 2012 at 12:40:17PM +1000, Benjamin Herrenschmidt wrote:
> > > On Mon, 2012-07-23 at 11:13 +0200, Uwe Kleine-König wrote:
> > > > This prepares *of_device_id.data becoming const. Without this change
> > > > the following warning would occur:
> > > >
> > > > drivers/macintosh/mediabay.c: In function 'media_bay_attach':
> > > > drivers/macintosh/mediabay.c:589:11: warning: assignment discards 'const' qualifier from pointer target type [enabled by default]
> > > >
> > > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > > > ---
> > >
> > > Ack all of these assuming you test built (I didn't).
> > "all" means the two mediabay patches? And yes, they are all built
> > tested.
>
> Yeah, anything powerpc you had in your series, but the two mediabay ones
> are what I spotted when digging through stuff today :-)
I added that to
serial/mpc52xx_uart: add a const qualifier
gpio/mpc8xxx: add a const qualifier
i2c/mpc: add a const qualifier
macintosh/mediabay: add a const qualifier
powerpc/83xx: add a const qualifier
powerpc/fsl_msi: add a const qualifier
powerpc/celleb_pci: add a const qualifier
watchdog/mpc8xxx: add a const qualifier
powerpc/fsl_msi: drop unneeded cast to non-const pointer
i2c/mpc: make data used as *of_device_id.data const
macintosh/mediabay: make data used as *of_device_id.data const
can: mpc5xxx_can: make data used as *of_device_id.data const
in the hope I didn't interpret "anything powerpc" too wide. Please tell
me if I did.
Thanks
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply
* Re: [PATCH v3 0/7] mv643xx.c: Add basic device tree support.
From: Benjamin Herrenschmidt @ 2012-09-11 6:03 UTC (permalink / raw)
To: Arnd Bergmann
Cc: thomas.petazzoni, andrew, netdev, devicetree-discuss,
linuxppc-dev, ben.dooks, Ian Molton, David Miller,
linux-arm-kernel
In-Reply-To: <201209101422.13875.arnd@arndb.de>
On Mon, 2012-09-10 at 14:22 +0000, Arnd Bergmann wrote:
> Following up on the old discussion, I talked briefly about this
> issue with BenH at the kernel summit. The outcome basically is that
> it's a bit sad to have incompatible bindings, but it's not the end
> of the world,and it's more important to do it right this time.
>
> Just make sure that you use different values for the 'compatible'
> strings and then do what you need to get the ARM hardware working.
>
> Ideally, the new binding should be written in a way that powerpc
> machines can use the same one, but the existing ones all use
> an version of Open Firmware that is not going to get updated
> and it's also not too likely that we are going to see new
> powerpc machines based on this chip.
Right, mostly these machines where the Pegasos. Those came with a fairly
busted variant of Open Firmware which generated a pretty gross
device-tree.
For some reason, the manufacturer of those things was never willing to
fix anything in their firmware (despite the distributor providing
patches etc...), seemingly on the assumption that whatever they were
doing was perfect and operating system people like us didn't matter one
little bit :-)
So I don't care much about it. It would be nice to keep them working
since people in the community still have them but if it goes through
some "compat" code that detects old/broken device-trees and eventually
disappears when we finally drop support, then so be it.
Cheers,
Ben.
^ permalink raw reply
* Re: [PATCH 2/6] powerpc: Add enable_ppr kernel parameter to enable PPR save/restore
From: Benjamin Herrenschmidt @ 2012-09-11 5:55 UTC (permalink / raw)
To: Haren Myneni; +Cc: paulus, Michael Neuling, linuxppc-dev, anton
In-Reply-To: <504ECF35.9070305@linux.vnet.ibm.com>
On Mon, 2012-09-10 at 22:42 -0700, Haren Myneni wrote:
>
> Thanks Michael. Yes, we noticed 6% overhead with null syscall test.
> Hence added cmdline option as suggested. I will add this comment in
> the
> changelog.
>
> Regarding the option name, I thought about various ones such as
> retain_process_ppr, retain_smt_priority, save_ppr and etc. Finally
> added
> 'enable_ppr' since it enables CPU_FTR (CPU_FTR_HAS_PPR) which allows
> to
> save/restore PPR value. Sure, I will change this option.
No, that isn't a problem with the name. It's a problem with the polarity
of the option.
If you need a command line argument to enable the option, then nobody
will enable it, it's pointless.
Cheers,
Ben.
^ permalink raw reply
* Re: [v3][PATCH 2/3] ppc/kprobe: complete kprobe and migrate exception frame
From: Benjamin Herrenschmidt @ 2012-09-11 5:51 UTC (permalink / raw)
To: Tiejun Chen; +Cc: linuxppc-dev
In-Reply-To: <1347330053-27039-2-git-send-email-tiejun.chen@windriver.com>
On Tue, 2012-09-11 at 10:20 +0800, Tiejun Chen wrote:
> We can't emulate stwu since that may corrupt current exception stack.
> So we will have to do real store operation in the exception return code.
>
> Firstly we'll allocate a trampoline exception frame below the kprobed
> function stack and copy the current exception frame to the trampoline.
> Then we can do this real store operation to implement 'stwu', and reroute
> the trampoline frame to r1 to complete this exception migration.
Ok, so not quite there yet :-)
See below:
> Signed-off-by: Tiejun Chen <tiejun.chen@windriver.com>
> ---
> arch/powerpc/kernel/entry_32.S | 45 ++++++++++++++++++++++++++++++++++------
> arch/powerpc/kernel/entry_64.S | 32 ++++++++++++++++++++++++++++
> 2 files changed, 71 insertions(+), 6 deletions(-)
>
> diff --git a/arch/powerpc/kernel/entry_32.S b/arch/powerpc/kernel/entry_32.S
> index ead5016..6cfe12f 100644
> --- a/arch/powerpc/kernel/entry_32.S
> +++ b/arch/powerpc/kernel/entry_32.S
> @@ -831,19 +831,54 @@ restore_user:
> bnel- load_dbcr0
> #endif
>
> -#ifdef CONFIG_PREEMPT
> b restore
>
> /* N.B. the only way to get here is from the beq following ret_from_except. */
> resume_kernel:
> + /* check current_thread_info, _TIF_EMULATE_STACK_STORE */
> + CURRENT_THREAD_INFO(r9, r1)
> + lwz r0,TI_FLAGS(r9)
> + andis. r0,r0,_TIF_EMULATE_STACK_STORE@h
> + beq+ 1f
So you used r0 to load the TI_FLAGS and immediately clobbered it in
andis. forcing you to re-load them later down. Instead, put them in r8
lwz r8,TI_FLAGS(r9)
andis. r0,r8,_TIF_*
beq+ *
> + addi r8,r1,INT_FRAME_SIZE /* Get the kprobed function entry */
Then you put your entry in r8 ....
> + lwz r3,GPR1(r1)
> + subi r3,r3,INT_FRAME_SIZE /* dst: Allocate a trampoline exception frame */
> + mr r4,r1 /* src: current exception frame */
> + li r5,INT_FRAME_SIZE /* size: INT_FRAME_SIZE */
> + mr r1,r3 /* Reroute the trampoline frame to r1 */
> + bl memcpy /* Copy from the original to the trampoline */
Which you just clobbered... oops :-)
So you need to store that old r1 somewhere fist then retrieve it
after the memcpy call. That or open-code the memcpy to avoid all
the clobbering problems.
> + CURRENT_THREAD_INFO(r9, r1)
> + lwz r0,TI_FLAGS(r9) /* Restore this clobbered r0 */
Re-load in r8 as suggested above ? Anyway, it doesn't matter you don't
actually need to load it at all because you re-load it in your
lwarx/stwcx. loop further down
> + /* Do real store operation to complete stwu */
> + lwz r5,GPR1(r1)
> + stw r8,0(r5)
(Storing a clobbered value.)
> + /* Clear _TIF_EMULATE_STACK_STORE flag */
> + CURRENT_THREAD_INFO(r9, r1)
Why re-calculate r9 here ? you just did 4 lines above
> + lis r11,_TIF_EMULATE_STACK_STORE@h
> + addi r5,r9,TI_FLAGS
> +0: lwarx r8,0,r5
>
> + andc r8,r8,r11
> +#ifdef CONFIG_IBM405_ERR77
> + dcbt 0,r5
> +#endif
> + stwcx. r8,0,r5
> + bne- 0b
So here, r8 contains TI_FLAGS
> +1:
And if you do the change I suggested above, here too.
> +#ifdef CONFIG_PREEMPT
> /* check current_thread_info->preempt_count */
> CURRENT_THREAD_INFO(r9, r1)
r9 already has what you want
> - lwz r0,TI_PREEMPT(r9)
> - cmpwi 0,r0,0 /* if non-zero, just restore regs and return */
> + lwz r8,TI_PREEMPT(r9)
> + cmpwi 0,r8,0 /* if non-zero, just restore regs and return */
Leave that to be r0, r8 has your TI_FLAGS already.
> bne restore
> - lwz r0,TI_FLAGS(r9)
See above.
> andi. r0,r0,_TIF_NEED_RESCHED
> beq+ restore
> + lwz r3,_MSR(r1)
> andi. r0,r3,MSR_EE /* interrupts off? */
> beq restore /* don't schedule if so */
> #ifdef CONFIG_TRACE_IRQFLAGS
> @@ -864,8 +899,6 @@ resume_kernel:
> */
> bl trace_hardirqs_on
> #endif
> -#else
> -resume_kernel:
> #endif /* CONFIG_PREEMPT */
>
> /* interrupts are hard-disabled at this point */
> diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
> index b40e0b4..b6d7483 100644
> --- a/arch/powerpc/kernel/entry_64.S
> +++ b/arch/powerpc/kernel/entry_64.S
> @@ -593,6 +593,38 @@ _GLOBAL(ret_from_except_lite)
> b .ret_from_except
>
> resume_kernel:
> + /* check current_thread_info, _TIF_EMULATE_STACK_STORE */
> + CURRENT_THREAD_INFO(r9, r1)
> + ld r0,TI_FLAGS(r9)
> + andis. r0,r0,_TIF_EMULATE_STACK_STORE@h
> + beq+ 1f
Similar comments to 32-bit
> + addi r8,r1,INT_FRAME_SIZE /* Get the kprobed function entry */
That gets clobbered too.
> + lwz r3,GPR1(r1)
> + subi r3,r3,INT_FRAME_SIZE /* dst: Allocate a trampoline exception frame */
> + mr r4,r1 /* src: current exception frame */
> + li r5,INT_FRAME_SIZE /* size: INT_FRAME_SIZE */
> + mr r1,r3 /* Reroute the trampoline frame to r1 */
> + bl memcpy /* Copy from the original to the trampoline */
> +
> + CURRENT_THREAD_INFO(r9, r1)
> + ld r4,TI_FLAGS(r9) /* Restore this clobbered r4 */
Usueless reloads
> + /* Do real store operation to complete stwu */
> + lwz r5,GPR1(r1)
> + std r8,0(r5)
> +
> + /* Clear _TIF_EMULATE_STACK_STORE flag */
> + CURRENT_THREAD_INFO(r9, r1)
> + lis r11,_TIF_EMULATE_STACK_STORE@h
> + addi r5,r9,TI_FLAGS
> +0: ldarx r8,0,r5
> + andc r8,r8,r11
> + stdcx. r8,0,r5
> + bne- 0b
> +1:
> +
> #ifdef CONFIG_PREEMPT
> /* Check if we need to preempt */
> andi. r0,r4,_TIF_NEED_RESCHED
Cheers,
Ben.
^ permalink raw reply
* Re: [PATCH 2/6] powerpc: Add enable_ppr kernel parameter to enable PPR save/restore
From: Haren Myneni @ 2012-09-11 5:42 UTC (permalink / raw)
To: Michael Neuling; +Cc: linuxppc-dev, anton, paulus
In-Reply-To: <13010.1347236558@neuling.org>
On 09/09/2012 05:22 PM, Michael Neuling wrote:
> Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:
>
>> On Sun, 2012-09-09 at 04:37 -0700, Haren Myneni wrote:
>>> enable_ppr kernel parameter is used to enable PPR save and restore.
>>> Supported on Power7 and later processors.
>>>
>>> By default, CPU_FTR_HAS_PPR is set for POWER7. If this parameter is not
>>> passed, disable CPU_FTR_HAS_PPR.
>>
>> What is the point ? Obscure / magic kernel command line options to turn
>> on a feature are pointless. Nobody knows about them, nobody enables
>> them.
>>
>> What you are doing is guarantee that nobody's ever going to enable your
>> code, so your whole patch series is thus irrelevant :-)
>>
>> If there's a good reason to *avoid* your option, then maybe consider
>> adding an option to *disable* the PPR save/restore, though of course
>> that would bring the argument that if it needs to be disabled maybe we
>> shouldn't do it in the first place, or you might want to think of a
>> reasonable way to intuit what the option should be.
>
> IIRC, Haren was saying there's a 6% hit on null syscall for this. So we
> suggested having a cmdline option to disable it for distros.
>
> Haren, is my recollection correct? If so, can you add this info the
> change log and change the sex of the option.
Thanks Michael. Yes, we noticed 6% overhead with null syscall test.
Hence added cmdline option as suggested. I will add this comment in the
changelog.
Regarding the option name, I thought about various ones such as
retain_process_ppr, retain_smt_priority, save_ppr and etc. Finally added
'enable_ppr' since it enables CPU_FTR (CPU_FTR_HAS_PPR) which allows to
save/restore PPR value. Sure, I will change this option.
Thanks
Haren
>
> Mikey
>
>>
>> Ben.
>>
>>> Signed-off-by: Haren Myneni <haren@us.ibm.com>
>>>
>>> ---
>>> Documentation/kernel-parameters.txt | 4 ++++
>>> arch/powerpc/include/asm/cputable.h | 6 ++++--
>>> arch/powerpc/kernel/setup_64.c | 14 ++++++++++++++
>>> 3 files changed, 22 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
>>> index ad7e2e5..2881e5f 100644
>>> --- a/Documentation/kernel-parameters.txt
>>> +++ b/Documentation/kernel-parameters.txt
>>> @@ -809,6 +809,10 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
>>> to discrete, to make X server driver able to add WB
>>> entry later. This parameter enables that.
>>>
>>> + enable_ppr [PPC/PSERIES]
>>> + Saves user defined PPR when process enters to kernel
>>> + and restores PPR at exit. But it impacts performance.
>>> +
>>> enable_timer_pin_1 [X86]
>>> Enable PIN 1 of APIC timer
>>> Can be useful to work around chipset bugs
>>> diff --git a/arch/powerpc/include/asm/cputable.h b/arch/powerpc/include/asm/cputable.h
>>> index b3c083d..880c469 100644
>>> --- a/arch/powerpc/include/asm/cputable.h
>>> +++ b/arch/powerpc/include/asm/cputable.h
>>> @@ -203,6 +203,7 @@ extern const char *powerpc_base_platform;
>>> #define CPU_FTR_POPCNTD LONG_ASM_CONST(0x0800000000000000)
>>> #define CPU_FTR_ICSWX LONG_ASM_CONST(0x1000000000000000)
>>> #define CPU_FTR_VMX_COPY LONG_ASM_CONST(0x2000000000000000)
>>> +#define CPU_FTR_HAS_PPR LONG_ASM_CONST(0x4000000000000000)
>>>
>>> #ifndef __ASSEMBLY__
>>>
>>> @@ -432,7 +433,8 @@ extern const char *powerpc_base_platform;
>>> CPU_FTR_PURR | CPU_FTR_SPURR | CPU_FTR_REAL_LE | \
>>> CPU_FTR_DSCR | CPU_FTR_SAO | CPU_FTR_ASYM_SMT | \
>>> CPU_FTR_STCX_CHECKS_ADDRESS | CPU_FTR_POPCNTB | CPU_FTR_POPCNTD | \
>>> - CPU_FTR_ICSWX | CPU_FTR_CFAR | CPU_FTR_HVMODE | CPU_FTR_VMX_COPY)
>>> + CPU_FTR_ICSWX | CPU_FTR_CFAR | CPU_FTR_HVMODE | \
>>> + CPU_FTR_VMX_COPY | CPU_FTR_HAS_PPR)
>>> #define CPU_FTRS_CELL (CPU_FTR_USE_TB | CPU_FTR_LWSYNC | \
>>> CPU_FTR_PPCAS_ARCH_V2 | CPU_FTR_CTRL | \
>>> CPU_FTR_ALTIVEC_COMP | CPU_FTR_MMCRA | CPU_FTR_SMT | \
>>> @@ -454,7 +456,7 @@ extern const char *powerpc_base_platform;
>>> (CPU_FTRS_POWER3 | CPU_FTRS_RS64 | CPU_FTRS_POWER4 | \
>>> CPU_FTRS_PPC970 | CPU_FTRS_POWER5 | CPU_FTRS_POWER6 | \
>>> CPU_FTRS_POWER7 | CPU_FTRS_CELL | CPU_FTRS_PA6T | \
>>> - CPU_FTR_VSX)
>>> + CPU_FTR_VSX | CPU_FTR_HAS_PPR)
>>> #endif
>>> #else
>>> enum {
>>> diff --git a/arch/powerpc/kernel/setup_64.c b/arch/powerpc/kernel/setup_64.c
>>> index 389bd4f..e4c1945 100644
>>> --- a/arch/powerpc/kernel/setup_64.c
>>> +++ b/arch/powerpc/kernel/setup_64.c
>>> @@ -98,6 +98,8 @@ int dcache_bsize;
>>> int icache_bsize;
>>> int ucache_bsize;
>>>
>>> +static u32 enable_ppr = 0;
>>> +
>>> #ifdef CONFIG_SMP
>>>
>>> static char *smt_enabled_cmdline;
>>> @@ -357,6 +359,9 @@ void __init setup_system(void)
>>> {
>>> DBG(" -> setup_system()\n");
>>>
>>> + if (cpu_has_feature(CPU_FTR_HAS_PPR) && !enable_ppr)
>>> + cur_cpu_spec->cpu_features &= ~CPU_FTR_HAS_PPR;
>>> +
>>> /* Apply the CPUs-specific and firmware specific fixups to kernel
>>> * text (nop out sections not relevant to this CPU or this firmware)
>>> */
>>> @@ -683,6 +688,15 @@ void __init setup_per_cpu_areas(void)
>>> }
>>> #endif
>>>
>>> +/* early_ppr kernel parameter to save/restore PPR register */
>>> +static int __init early_ppr_enabled(char *str)
>>> +{
>>> + enable_ppr = 1;
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +early_param("enable_ppr", early_ppr_enabled);
>>>
>>> #ifdef CONFIG_PPC_INDIRECT_IO
>>> struct ppc_pci_io ppc_pci_io;
>>
>>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
>
^ permalink raw reply
* Re: [PATCH 5/6] powerpc: Macros for saving/restore PPR
From: Benjamin Herrenschmidt @ 2012-09-11 5:37 UTC (permalink / raw)
To: Haren Myneni; +Cc: linuxppc-dev, mikey, paulus, anton
In-Reply-To: <504EC99B.7000302@linux.vnet.ibm.com>
On Mon, 2012-09-10 at 22:18 -0700, Haren Myneni wrote:
> We can also use thread struct, but the saved ppr is needed as long as
> the process is in kernel context. Once the process exits, we do not need
> this value - means the kernel or any other command will not be reading
> this value.
>
> Even the process can change this value later. Need to add
> ppr in thread_struct which uses extra 8 bytes for each task.
Allright so the real answer is that the thread info is easier to get at
than the thread struct :-) However, the most logical place to put that
stuff is along the lines of the surrounding code... in the pt_regs on
the stack.
It should be fairly easy to carve some space here, for example softe
doesn't have to be 64-bit (it's really one bit, you can make it one
byte, just make sure to adapt the corresponding assembly).
The main issue with that approach is that you then need to audit a bit
to see how pt_regs might be constructed here or there in case you end up
with a 0 (illegal PPR) value in there.
> Whereas thread_info is at the top of stack. No need to allocate extra
> memory. Since the saved process PPR value is needed temporarily whenever
> the process is in kernel, thought thread_info is proper. Also easy to
> read - one less load instruction.
The advantage of pt_regs is that a signal can "peek" at the PPR value
which could be useful for debugging purposes.
> Saving in thread_info:
>
> ld r4,area+EX_PPR(r13);
> clrrdi r5,r1,THREAD_SHIFT;
> std r4,TI_PPR(r5);
>
> Saving in thread_struct:
>
> ld r4,PACACURRENT(r13)
> addi r5,r4,THREAD
> ld r4,paca+EX_PPR(r13);
> std r4,THREAD_PPR(r5)
>
> If you prefer thread_struct, I will change the patch. Please let me know.
Cheers,
Ben.
> Thanks
> Haren
>
> >
> > Cheers,
> > Ben.
> >
> >> +#define RESTORE_PPR(ra,rb) \
> >> +BEGIN_FTR_SECTION_NESTED(941) \
> >> + clrrdi ra,r1,THREAD_SHIFT; \
> >> + ld rb,TI_PPR(ra); /* Read PPR from thread_info */ \
> >> + mtspr SPRN_PPR,rb; /* Restore PPR */ \
> >> +END_FTR_SECTION_NESTED(CPU_FTR_HAS_PPR,CPU_FTR_HAS_PPR,941)
> >> +
> >> +#define RESTORE_PPR_PACA(area,ra) \
> >> +BEGIN_FTR_SECTION_NESTED(942) \
> >> + ld ra,area+EX_PPR(r13); \
> >> + mtspr SPRN_PPR,ra; \
> >> +END_FTR_SECTION_NESTED(CPU_FTR_HAS_PPR,CPU_FTR_HAS_PPR,942)
> >> +
> >> +#define HMT_MEDIUM_NO_PPR \
> >> +BEGIN_FTR_SECTION_NESTED(944) \
> >> + HMT_MEDIUM; \
> >> +END_FTR_SECTION_NESTED(CPU_FTR_HAS_PPR,0,944) /*non P7*/
> >> +
> >> +#define HMT_MEDIUM_HAS_PPR(area, ra) \
> >> +BEGIN_FTR_SECTION_NESTED(943) \
> >> + mfspr ra,SPRN_PPR; \
> >> + std ra,area+EX_PPR(r13); \
> >> + HMT_MEDIUM; \
> >> +END_FTR_SECTION_NESTED(CPU_FTR_HAS_PPR,CPU_FTR_HAS_PPR,943) /* P7 */
> >> +
> >> #define __EXCEPTION_PROLOG_1(area, extra, vec) \
> >> GET_PACA(r13); \
> >> std r9,area+EX_R9(r13); /* save r9 - r12 */ \
> >> diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h
> >> index 6386086..dff2f89 100644
> >> --- a/arch/powerpc/include/asm/reg.h
> >> +++ b/arch/powerpc/include/asm/reg.h
> >> @@ -284,6 +284,7 @@
> >> #define SPRN_DBAT6U 0x23C /* Data BAT 6 Upper Register */
> >> #define SPRN_DBAT7L 0x23F /* Data BAT 7 Lower Register */
> >> #define SPRN_DBAT7U 0x23E /* Data BAT 7 Upper Register */
> >> +#define SPRN_PPR 0x380 /* SMT Thread status Register */
> >>
> >> #define SPRN_DEC 0x016 /* Decrement Register */
> >> #define SPRN_DER 0x095 /* Debug Enable Regsiter */
> >
> >
^ permalink raw reply
* Re: [RFC v8 PATCH 00/20] memory-hotplug: hot-remove physical memory
From: Wen Congyang @ 2012-09-11 5:39 UTC (permalink / raw)
To: Jerry
Cc: linux-s390, linux-ia64, linux-acpi, len.brown, linux-mm, linux-sh,
x86, linux-kernel, cmetcalf, Vasilis Liaskovitis, Minchan Kim,
Yasuaki Ishimatsu, paulus, kosaki.motohiro, rientjes, sparclinux,
Andrew Morton, linuxppc-dev, cl, liuj97
In-Reply-To: <CAAV+Mu4hb0qbW2Ry6w5FAGUM06puDH0v_H-jr584-G9CzJqSGw@mail.gmail.com>
At 09/11/2012 01:18 PM, Jerry Wrote:
> Hi Kim,
>
> Thank you for your kindness. Let me clarify this:
>
> On ARM architecture, there are 32 bits physical addresses space. However,
> the addresses space is divided into 8 banks normally. Each bank
> disabled/enabled by a chip selector signal. In my platform, bank0 connects
> a DDR chip, and bank1 also connects another DDR chip. And each DDR chip
> whose capability is 512MB is integrated into the main board. So, it could
> not be removed by hand. We can disable/enable each bank by peripheral
> device controller registers.
>
> When system enter suspend state, if all the pages allocated could be
> migrated to one bank, there are no valid data in the another bank. In this
> time, I could disable the free bank. It isn't necessary to provided power
> to this chip in the suspend state. When system resume, I just need to
> enable it again.
>
> Hi Wen,
>
> I am sorry for that I doesn't know the "_PSx support" means. Maybe I
> needn't it.
Hmm, arm doesn't support ACPI, so please ignore it.
Thanks
Wen Congyang
>
> Thanks,
> Jerry
>
> 2012/9/11 Minchan Kim <minchan@kernel.org>
>
>> Hi Jerry,
>>
>> On Tue, Sep 11, 2012 at 08:27:40AM +0800, Jerry wrote:
>>> Hi Wen,
>>>
>>> I have been arranged a job related memory hotplug on ARM architecture.
>>> Maybe I know some new issues about memory hotplug on ARM architecture. I
>>> just enabled it on ARM, and it works well in my Android tablet now.
>>> However, I have not send out my patches. The real reason is that I don't
>>> know how to do it. Maybe I need to read
>> "Documentation/SubmittingPatches".
>>>
>>> Hi Andrew,
>>> This is my first time to send you a e-mail. I am so nervous about if I
>> have
>>> some mistakes or not.
>>
>> Don't be afraid.
>> If you might make a mistake, it's very natural to newbie.
>> I am sure anyone doesn't blame you. :)
>> If you have a good patch, please send out.
>>
>>>
>>> Some peoples maybe think memory hotplug need to be supported by special
>>> hardware. Maybe it means memory physical hotplug. Some times, we just
>> need
>>> to use memory logical hotplug, doesn't remove the memory in physical. It
>> is
>>> also usefully for power saving in my platform. Because I doesn't want
>>> the offline memory is in *self-refresh* state.
>>
>> Just out of curiosity.
>> What's the your scenario and gain?
>> AFAIK, there were some effort about it in embedded side but gain isn't
>> rather big
>> IIRC.
>>
>>>
>>> Any comments are appreciated.
>>>
>>> Thanks,
>>> Jerry
>>>
>>> 2012/9/10 Vasilis Liaskovitis <vasilis.liaskovitis@profitbricks.com>
>>>
>>>> Hi,
>>>>
>>>> On Mon, Sep 10, 2012 at 10:01:44AM +0800, Wen Congyang wrote:
>>>>> At 09/10/2012 09:46 AM, Yasuaki Ishimatsu Wrote:
>>>>>> Hi Wen,
>>>>>>
>>>>>> 2012/09/01 5:49, Andrew Morton wrote:
>>>>>>> On Tue, 28 Aug 2012 18:00:07 +0800
>>>>>>> wency@cn.fujitsu.com wrote:
>>>>>>>
>>>>>>>> This patch series aims to support physical memory hot-remove.
>>>>>>>
>>>>>>> I doubt if many people have hardware which permits physical memory
>>>>>>> removal? How would you suggest that people with regular hardware
>> can
>>>>>>> test these chagnes?
>>>>>>
>>>>>> How do you test the patch? As Andrew says, for hot-removing memory,
>>>>>> we need a particular hardware. I think so too. So many people may
>> want
>>>>>> to know how to test the patch.
>>>>>> If we apply following patch to kvm guest, can we hot-remove memory
>> on
>>>>>> kvm guest?
>>>>>>
>>>>>> http://lists.gnu.org/archive/html/qemu-devel/2012-07/msg01389.html
>>>>>
>>>>> Yes, if we apply this patchset, we can test hot-remove memory on kvm
>>>> guest.
>>>>> But that patchset doesn't implement _PS3, so there is some
>> restriction.
>>>>
>>>> the following repos contain the patchset above, plus 2 more patches
>> that
>>>> add
>>>> PS3 support to the dimm devices in qemu/seabios:
>>>>
>>>> https://github.com/vliaskov/seabios/commits/memhp-v2
>>>> https://github.com/vliaskov/qemu-kvm/commits/memhp-v2
>>>>
>>>> I have not posted the PS3 patches yet in the qemu list, but will post
>> them
>>>> soon for v3 of the memory hotplug series. If you have issues testing,
>> let
>>>> me
>>>> know.
>>>>
>>>> thanks,
>>>>
>>>> - Vasilis
>>>>
>>>> --
>>>> To unsubscribe, send a message with 'unsubscribe linux-mm' in
>>>> the body to majordomo@kvack.org. For more info on Linux MM,
>>>> see: http://www.linux-mm.org/ .
>>>> Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
>>>>
>>>
>>>
>>>
>>> --
>>> I love linux!!!
>>
>> --
>> Kind regards,
>> Minchan Kim
>>
>
>
>
^ permalink raw reply
* Re: [PATCH 1/6] powerpc: Move branch instruction from ACCOUNT_CPU_USER_ENTRY to caller
From: Haren Myneni @ 2012-09-11 5:23 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: paulus, mikey, linuxppc-dev, anton
In-Reply-To: <1347235505.2385.134.camel@pasglop>
On 09/09/2012 05:05 PM, Benjamin Herrenschmidt wrote:
> On Sun, 2012-09-09 at 04:36 -0700, Haren Myneni wrote:
>> The first instruction in ACCOUNT_CPU_USER_ENTRY is 'beq' which checkes for
>> exceptions coming from kernel mode. PPR value will be saved immediately after
>> ACCOUNT_CPU_USER_ENTRY and is also for user level exceptions. So moved this
>> branch instruction in the caller code.
>
> grep fail ? ACCOUNT_CPU_USER_ENTRY is used in exception-64e.S as well,
> so that needs to be updated too.
Sorry, I will make this change.
>
> Cheers,
> Ben.
>
>> Signed-off-by: Haren Myneni <haren@us.ibm.com>
>>
>> ---
>> arch/powerpc/include/asm/exception-64s.h | 3 ++-
>> arch/powerpc/include/asm/ppc_asm.h | 2 --
>> arch/powerpc/kernel/entry_64.S | 3 ++-
>> 3 files changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/powerpc/include/asm/exception-64s.h b/arch/powerpc/include/asm/exception-64s.h
>> index a43c147..45702e0 100644
>> --- a/arch/powerpc/include/asm/exception-64s.h
>> +++ b/arch/powerpc/include/asm/exception-64s.h
>> @@ -176,8 +176,9 @@ do_kvm_##n: \
>> std r10,0(r1); /* make stack chain pointer */ \
>> std r0,GPR0(r1); /* save r0 in stackframe */ \
>> std r10,GPR1(r1); /* save r1 in stackframe */ \
>> + beq 4f; /* if from kernel mode */ \
>> ACCOUNT_CPU_USER_ENTRY(r9, r10); \
>> - std r2,GPR2(r1); /* save r2 in stackframe */ \
>> +4: std r2,GPR2(r1); /* save r2 in stackframe */ \
>> SAVE_4GPRS(3, r1); /* save r3 - r6 in stackframe */ \
>> SAVE_2GPRS(7, r1); /* save r7, r8 in stackframe */ \
>> ld r9,area+EX_R9(r13); /* move r9, r10 to stackframe */ \
>> diff --git a/arch/powerpc/include/asm/ppc_asm.h b/arch/powerpc/include/asm/ppc_asm.h
>> index ea2a86e..376e36d 100644
>> --- a/arch/powerpc/include/asm/ppc_asm.h
>> +++ b/arch/powerpc/include/asm/ppc_asm.h
>> @@ -30,7 +30,6 @@
>> #define ACCOUNT_STOLEN_TIME
>> #else
>> #define ACCOUNT_CPU_USER_ENTRY(ra, rb) \
>> - beq 2f; /* if from kernel mode */ \
>> MFTB(ra); /* get timebase */ \
>> ld rb,PACA_STARTTIME_USER(r13); \
>> std ra,PACA_STARTTIME(r13); \
>> @@ -38,7 +37,6 @@
>> ld ra,PACA_USER_TIME(r13); \
>> add ra,ra,rb; /* add on to user time */ \
>> std ra,PACA_USER_TIME(r13); \
>> -2:
>>
>> #define ACCOUNT_CPU_USER_EXIT(ra, rb) \
>> MFTB(ra); /* get timebase */ \
>> diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
>> index b40e0b4..8d21cc4 100644
>> --- a/arch/powerpc/kernel/entry_64.S
>> +++ b/arch/powerpc/kernel/entry_64.S
>> @@ -62,8 +62,9 @@ system_call_common:
>> std r12,_MSR(r1)
>> std r0,GPR0(r1)
>> std r10,GPR1(r1)
>> + beq 2f /* if from kernel mode */
>> ACCOUNT_CPU_USER_ENTRY(r10, r11)
>> - std r2,GPR2(r1)
>> +2: std r2,GPR2(r1)
>> std r3,GPR3(r1)
>> mfcr r2
>> std r4,GPR4(r1)
>
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox