* Re: [PATCH v2 12/12] x86/traps: Fix up invalid PASID
From: Fenghua Yu @ 2020-06-16 23:23 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Dave Hansen, H Peter Anvin, Dave Jiang, Ashok Raj, Joerg Roedel,
x86, amd-gfx, Ingo Molnar, Ravi V Shankar, Yu-cheng Yu,
Andrew Donnellan, Borislav Petkov, Sohil Mehta, Thomas Gleixner,
Tony Luck, linuxppc-dev, Felix Kuehling, linux-kernel, iommu,
Jacob Jun Pan, Frederic Barrat, David Woodhouse, Lu Baolu
In-Reply-To: <20200615190928.GJ2531@hirez.programming.kicks-ass.net>
Hi, Peter,
On Mon, Jun 15, 2020 at 09:09:28PM +0200, Peter Zijlstra wrote:
> On Mon, Jun 15, 2020 at 11:55:29AM -0700, Fenghua Yu wrote:
>
> > Or do you suggest to add a random new flag in struct thread_info instead
> > of a TIF flag?
>
> Why thread_info? What's wrong with something simple like the below. It
> takes a bit from the 'strictly current' flags word.
>
>
> diff --git a/include/linux/sched.h b/include/linux/sched.h
> index b62e6aaf28f0..fca830b97055 100644
> --- a/include/linux/sched.h
> +++ b/include/linux/sched.h
> @@ -801,6 +801,9 @@ struct task_struct {
> /* Stalled due to lack of memory */
> unsigned in_memstall:1;
> #endif
> +#ifdef CONFIG_PCI_PASID
> + unsigned has_valid_pasid:1;
> +#endif
>
> unsigned long atomic_flags; /* Flags requiring atomic access. */
>
> diff --git a/kernel/fork.c b/kernel/fork.c
> index 142b23645d82..10b3891be99e 100644
> --- a/kernel/fork.c
> +++ b/kernel/fork.c
> @@ -955,6 +955,10 @@ static struct task_struct *dup_task_struct(struct task_struct *orig, int node)
> tsk->use_memdelay = 0;
> #endif
>
> +#ifdef CONFIG_PCI_PASID
> + tsk->has_valid_pasid = 0;
> +#endif
> +
> #ifdef CONFIG_MEMCG
> tsk->active_memcg = NULL;
> #endif
Can I add "Signed-off-by: Peter Zijlstra <peterz@infradead.org>"
to this patch? I will send this patch in the next version of the series.
Thanks.
-Fenghua
^ permalink raw reply
* Re: [PATCH 2/2] ASoC: fsl_spdif: Add support for imx6sx platform
From: Nicolin Chen @ 2020-06-16 23:28 UTC (permalink / raw)
To: Shengjiu Wang
Cc: devicetree, alsa-devel, timur, Xiubo.Lee, lgirdwood, linuxppc-dev,
tiwai, robh+dt, perex, broonie, festevam, linux-kernel
In-Reply-To: <1592289761-29118-2-git-send-email-shengjiu.wang@nxp.com>
On Tue, Jun 16, 2020 at 02:42:41PM +0800, Shengjiu Wang wrote:
> The one difference on imx6sx platform is that the root clock
> is shared with ASRC module, so we add a new flags "ind_root_clk"
> which means the root clock is independent, then we will not
> do the clk_set_rate and clk_round_rate to avoid impact ASRC
> module usage.
>
> As add a new flags, we include the soc specific data struct.
>
> Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
> ---
> sound/soc/fsl/fsl_spdif.c | 43 +++++++++++++++++++++++++++++++++++----
> 1 file changed, 39 insertions(+), 4 deletions(-)
>
> diff --git a/sound/soc/fsl/fsl_spdif.c b/sound/soc/fsl/fsl_spdif.c
> index 1b2e516f9162..00e06803d32f 100644
> --- a/sound/soc/fsl/fsl_spdif.c
> +++ b/sound/soc/fsl/fsl_spdif.c
> @@ -42,6 +42,17 @@ static u8 srpc_dpll_locked[] = { 0x0, 0x1, 0x2, 0x3, 0x4, 0xa, 0xb };
>
> #define DEFAULT_RXCLK_SRC 1
>
> +/**
> + * struct fsl_spdif_soc_data: soc specific data
> + *
> + * @imx: for imx platform
> + * @ind_root_clk: flag for round clk rate
> + */
> +struct fsl_spdif_soc_data {
> + bool imx;
> + bool ind_root_clk;
"ind" doesn't look very straightforward; maybe "shared_root_clock"?
And for its comments:
* @shared_root_clock: flag of sharing a clock source with others;
* so the driver shouldn't set root clock rate
> +};
> +
> /*
> * SPDIF control structure
> * Defines channel status, subcode and Q sub
> @@ -93,6 +104,7 @@ struct fsl_spdif_priv {
> struct snd_soc_dai_driver cpu_dai_drv;
> struct platform_device *pdev;
> struct regmap *regmap;
> + const struct fsl_spdif_soc_data *soc;
Looks better if we move it to the top of the list :)
> @@ -421,7 +448,7 @@ static int spdif_set_sample_rate(struct snd_pcm_substream *substream,
> sysclk_df = spdif_priv->sysclk_df[rate];
>
> /* Don't mess up the clocks from other modules */
> - if (clk != STC_TXCLK_SPDIF_ROOT)
> + if (clk != STC_TXCLK_SPDIF_ROOT || !spdif_priv->soc->ind_root_clk)
> goto clk_set_bypass;
>
> /* The S/PDIF block needs a clock of 64 * fs * txclk_df */
> @@ -1186,7 +1213,8 @@ static int fsl_spdif_probe_txclk(struct fsl_spdif_priv *spdif_priv,
> continue;
>
> ret = fsl_spdif_txclk_caldiv(spdif_priv, clk, savesub, index,
> - i == STC_TXCLK_SPDIF_ROOT);
> + i == STC_TXCLK_SPDIF_ROOT &&
> + spdif_priv->soc->ind_root_clk);
Having more than one place that checks the condition, we can add:
/* Check if clk is a root clock that does not share clock source with others */
static inline bool fsl_spdif_can_set_clk_rate(struct fsl_spdif_priv *spdif, int clk)
{
return (clk == STC_TXCLK_SPDIF_ROOT) && !spdif->soc->shared_root_clock;
}
^ permalink raw reply
* Re: [PATCH kernel] powerpc/powernv/ioda: Return correct error if TCE level allocation failed
From: Michael Ellerman @ 2020-06-16 23:37 UTC (permalink / raw)
To: Alexey Kardashevskiy, linuxppc-dev; +Cc: Alexey Kardashevskiy
In-Reply-To: <20200616104231.27805-1-aik@ozlabs.ru>
Alexey Kardashevskiy <aik@ozlabs.ru> writes:
> The iommu_table_ops::xchg_no_kill() callback updates TCE. It is quite
> possible that not entire table is allocated if it is huge and multilevel
> so xchg may also allocate subtables. If failed, it returns H_HARDWARE
> for failed allocation and H_TOO_HARD if it needs it but cannot do because
> the alloc parameter is "false" (set when called with MMU=off to force
> retry with MMU=on).
>
> The problem is that having separate errors only matters in real mode
> (MMU=off) but the only caller with alloc="false" does not check the exact
> error code and simply returns H_TOO_HARD; and for every other mode
> alloc is "true". Also, the function is also called from the ioctl()
> handler of the VFIO SPAPR TCE IOMMU subdriver which does not expect
> hypervisor error codes (H_xxx) and will expose them to the userspace.
>
> This converts wrong error codes to a simple -1.
>
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
>
> I could make it "return alloc ? -ENOMEM : -EBUSY" but
> is EBUSY a good match for H_TOO_HARD?
I think -EAGAIN would be the best match.
But it would be simpler if it just returned -ENOMEM always. In both
cases the problem is that the function needs to allocate memory but
couldn't.
If a caller passes alloc=false, it knows that, so if it sees ENOMEM it
can retry with alloc=true.
cheers
^ permalink raw reply
* Re: [PATCH v4 0/3] mm, treewide: Rename kzfree() to kfree_sensitive()
From: Matthew Wilcox @ 2020-06-17 0:37 UTC (permalink / raw)
To: dsterba, Joe Perches, Waiman Long, Andrew Morton, David Howells,
Jarkko Sakkinen, James Morris, Serge E. Hallyn, Linus Torvalds,
David Rientjes, Michal Hocko, Johannes Weiner, Dan Carpenter,
Jason A . Donenfeld, linux-mm, keyrings, linux-kernel,
linux-crypto, linux-pm, linux-stm32, linux-amlogic,
linux-mediatek, linuxppc-dev, virtualization, netdev, linux-ppp,
wireguard, linux-wireless, devel, linux-scsi, target-devel,
linux-btrfs, linux-cifs, linux-fscrypt, ecryptfs, kasan-dev,
linux-bluetooth, linux-wpan, linux-sctp, linux-nfs,
tipc-discussion, linux-security-module, linux-integrity
In-Reply-To: <20200616230130.GJ27795@twin.jikos.cz>
On Wed, Jun 17, 2020 at 01:01:30AM +0200, David Sterba wrote:
> On Tue, Jun 16, 2020 at 11:53:50AM -0700, Joe Perches wrote:
> > On Mon, 2020-06-15 at 21:57 -0400, Waiman Long wrote:
> > > v4:
> > > - Break out the memzero_explicit() change as suggested by Dan Carpenter
> > > so that it can be backported to stable.
> > > - Drop the "crypto: Remove unnecessary memzero_explicit()" patch for
> > > now as there can be a bit more discussion on what is best. It will be
> > > introduced as a separate patch later on after this one is merged.
> >
> > To this larger audience and last week without reply:
> > https://lore.kernel.org/lkml/573b3fbd5927c643920e1364230c296b23e7584d.camel@perches.com/
> >
> > Are there _any_ fastpath uses of kfree or vfree?
>
> I'd consider kfree performance critical for cases where it is called
> under locks. If possible the kfree is moved outside of the critical
> section, but we have rbtrees or lists that get deleted under locks and
> restructuring the code to do eg. splice and free it outside of the lock
> is not always possible.
Not just performance critical, but correctness critical. Since kvfree()
may allocate from the vmalloc allocator, I really think that kvfree()
should assert that it's !in_atomic(). Otherwise we can get into trouble
if we end up calling vfree() and have to take the mutex.
^ permalink raw reply
* Re: powerpc/pci: [PATCH 1/1 V3] PCIE PHB reset
From: Oliver O'Halloran @ 2020-06-17 0:45 UTC (permalink / raw)
To: Michael Ellerman; +Cc: Brian King, Wen Xiong, linuxppc-dev, wenxiong
In-Reply-To: <87r1ufdy1x.fsf@mpe.ellerman.id.au>
On Tue, Jun 16, 2020 at 9:55 PM Michael Ellerman <mpe@ellerman.id.au> wrote:
>
> wenxiong@linux.vnet.ibm.com writes:
> > From: Wen Xiong <wenxiong@linux.vnet.ibm.com>
> >
> > Several device drivers hit EEH(Extended Error handling) when triggering
> > kdump on Pseries PowerVM. This patch implemented a reset of the PHBs
> > in pci general code when triggering kdump.
>
> Actually it's in pseries specific PCI code, and the reset is done in the
> 2nd kernel as it boots, not when triggering the kdump.
>
> You're doing it as a:
>
> machine_postcore_initcall(pseries, pseries_phb_reset);
>
> But we do the EEH initialisation in:
>
> core_initcall_sync(eeh_init);
>
> Which happens first.
>
> So it seems to me that this should be called from pseries_eeh_init().
This happens to use some of the same RTAS calls as EEH, but it's
entirely orthogonal to it. Wedging the two together doesn't make any
real sense IMO since this should be usable even with !CONFIG_EEH.
> That would isolate the code in the right place, and allow you to use the
> existing ibm_get_config_addr_info.
>
> You probably can't use pseries_eeh_get_pe_addr(), because you won't have
> a "pe" structure yet.
>
> Instead you should add a helper that does the core of that logic but
> accepts config_addr/buid as parameters, and then have your code and
> pseries_eeh_get_pe_addr() call that.
I have a patch in my next pile of eeh reworks which kills off
pseries_eeh_get_pe_addr() entirely. It's used to implement
eeh_ops->get_pe_addr for pseries, but the only caller of
->get_pe_addr() is in pseries EEH code and the powernv version is a
stub so I was going to drop it from EEH ops and fold it into the
caller. We could do that in this patch, but it's just going to create
a merge conflict for you to deal with. Up to you.
> *snip*
> > + list_for_each_entry(phb, &hose_list, list_node) {
> > + config_addr = pseries_get_pdn_addr(phb);
> > + if (config_addr == -1)
> > + continue;
> > +
> > + ret = rtas_call(ibm_set_slot_reset, 4, 1, NULL,
> > + config_addr, BUID_HI(phb->buid),
> > + BUID_LO(phb->buid), EEH_RESET_FUNDAMENTAL);
> > +
> > + /* If fundamental-reset not supported, try hot-reset */
> > + if (ret == -8)
>
> Where does -8 come from?
There's a comment right there.
It could be better explained I suppose, but you need to read
PAPR/LoPAPR to make sense of anything RTAS so what's it really buying
you?
> Oh I see, it's copied from pseries_eeh_reset().
> > + ret = rtas_call(ibm_set_slot_reset, 4, 1, NULL,
> > + config_addr, BUID_HI(phb->buid),
> > + BUID_LO(phb->buid), EEH_RESET_HOT);
> > +
> > + if (ret) {
> > + pr_err("%s: PHB#%x-PE# failed with rtas_call activate reset=%d\n",
> ^
> again missing PE number.
>
> > + __func__, phb->global_number, ret);
> > + continue;
> > + }
> > + }
> > + msleep(EEH_PE_RST_SETTLE_TIME);
>
> So that loop is basically a copy of pseries_eeh_reset() but with the
> sleep hoisted out of the loop.
>
> I'd really prefer to see that refactored into a helper that takes the
> config_addr and buid and doesn't do the sleep.
>
> Then this loop could call that helper, and so could pseries_eeh_reset().
That's better so long as we're not requiring CONFIG_EEH being
selected. pseries_eeh_reset() uses the rtas token variables
initialised in pseries_eeh_init() which are static to the file so
they'd need to be initialised somewhere common.
^ permalink raw reply
* Re: [PATCH 2/2] ASoC: fsl-asoc-card: Add MQS support
From: Nicolin Chen @ 2020-06-17 0:48 UTC (permalink / raw)
To: Shengjiu Wang
Cc: devicetree, alsa-devel, timur, Xiubo.Lee, lgirdwood, linuxppc-dev,
tiwai, robh+dt, perex, broonie, festevam, linux-kernel
In-Reply-To: <1592292637-25734-2-git-send-email-shengjiu.wang@nxp.com>
On Tue, Jun 16, 2020 at 03:30:37PM +0800, Shengjiu Wang wrote:
> The MQS codec isn't an i2c device, so add a new platform device for it.
>
> MQS only support playback, so add a new audio map.
>
> Add there maybe "model" property or no "audio-routing" property insertions
"Add" => "And"
> devicetree, so add some enhancement for these two property.
>
> Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
> ---
> sound/soc/fsl/fsl-asoc-card.c | 70 ++++++++++++++++++++++++++---------
> 1 file changed, 52 insertions(+), 18 deletions(-)
>
> diff --git a/sound/soc/fsl/fsl-asoc-card.c b/sound/soc/fsl/fsl-asoc-card.c
> index 00be73900888..2ac8cb9ddd10 100644
> --- a/sound/soc/fsl/fsl-asoc-card.c
> +++ b/sound/soc/fsl/fsl-asoc-card.c
> @@ -482,6 +489,7 @@ static int fsl_asoc_card_probe(struct platform_device *pdev)
> {
> struct device_node *cpu_np, *codec_np, *asrc_np;
> struct device_node *np = pdev->dev.of_node;
> + struct platform_device *codec_pdev = NULL; /* used for non i2c device*/
Having both codec_pdev and codec_dev duplicates things. Actually
only a couple of places really need "codec_dev" -- most of them
need codec_dev->dev pointer instead. So we could have a cleanup:
- struct i2c_client *codec_dev;
+ struct device *codec_dev = NULL;
> @@ -512,10 +520,13 @@ static int fsl_asoc_card_probe(struct platform_device *pdev)
> }
>
> codec_np = of_parse_phandle(np, "audio-codec", 0);
> - if (codec_np)
> + if (codec_np) {
> codec_dev = of_find_i2c_device_by_node(codec_np);
> - else
> + if (!codec_dev)
> + codec_pdev = of_find_device_by_node(codec_np);
> + } else {
> codec_dev = NULL;
> + }
Here can have something like (feel free to simplify):
if (codec_np) {
struct platform_device *codec_pdev;
struct i2c_client *codec_i2c;
codec_i2c = of_find_i2c_device_by_node(codec_np);
if (codec_i2c)
codec_dev = &codec_i2c->dev;
if (!codec_dev) {
codec_pdev = of_find_device_by_node(codec_np);
codec_dev = &codec_pdev->dev;
}
}
> asrc_np = of_parse_phandle(np, "audio-asrc", 0);
> if (asrc_np)
> @@ -525,6 +536,13 @@ static int fsl_asoc_card_probe(struct platform_device *pdev)
> if (codec_dev) {
> struct clk *codec_clk = clk_get(&codec_dev->dev, NULL);
Then here:
- struct clk *codec_clk = clk_get(&codec_dev->dev, NULL);
+ struct clk *codec_clk = clk_get(codec_dev, NULL);
> @@ -538,6 +556,11 @@ static int fsl_asoc_card_probe(struct platform_device *pdev)
> /* Assign a default DAI format, and allow each card to overwrite it */
> priv->dai_fmt = DAI_FMT_BASE;
>
> + memcpy(priv->dai_link, fsl_asoc_card_dai,
> + sizeof(struct snd_soc_dai_link) * ARRAY_SIZE(priv->dai_link));
> @@ -573,13 +596,25 @@ static int fsl_asoc_card_probe(struct platform_device *pdev)
> codec_dai_name = "ac97-hifi";
> priv->card.set_bias_level = NULL;
> priv->dai_fmt = SND_SOC_DAIFMT_AC97;
> + priv->card.dapm_routes = audio_map_ac97;
> + priv->card.num_dapm_routes = ARRAY_SIZE(audio_map_ac97);
> + } else if (of_device_is_compatible(np, "fsl,imx-audio-mqs")) {
> + codec_dai_name = "fsl-mqs-dai";
> + priv->card.set_bias_level = NULL;
> + priv->dai_fmt = SND_SOC_DAIFMT_LEFT_J |
> + SND_SOC_DAIFMT_CBS_CFS |
> + SND_SOC_DAIFMT_NB_NF;
> + priv->dai_link[1].dpcm_playback = 1;
> + priv->dai_link[2].dpcm_playback = 1;
dpcm_playback = 1? That's the default value in fsl_asoc_card_dai.
> @@ -601,19 +636,18 @@ static int fsl_asoc_card_probe(struct platform_device *pdev)
> priv->cpu_priv.sysclk_id[0] = FSL_SAI_CLK_MAST1;
> }
>
> - snprintf(priv->name, sizeof(priv->name), "%s-audio",
> - fsl_asoc_card_is_ac97(priv) ? "ac97" :
> - codec_dev->name);
> -
> /* Initialize sound card */
> priv->pdev = pdev;
> priv->card.dev = &pdev->dev;
> - priv->card.name = priv->name;
> + ret = snd_soc_of_parse_card_name(&priv->card, "model");
> + if (ret) {
> + snprintf(priv->name, sizeof(priv->name), "%s-audio",
> + fsl_asoc_card_is_ac97(priv) ? "ac97" :
> + (codec_dev ? codec_dev->name : codec_pdev->name));
We can just use dev_name() if codec_dev is struct device *
Or having a codec_dev_name to cache codec_pdev/i2c->name.
> - ret = snd_soc_of_parse_audio_routing(&priv->card, "audio-routing");
> - if (ret) {
> - dev_err(&pdev->dev, "failed to parse audio-routing: %d\n", ret);
> - goto asrc_fail;
> + if (of_property_read_bool(np, "audio-routing")) {
> + ret = snd_soc_of_parse_audio_routing(&priv->card, "audio-routing");
> + if (ret) {
> + dev_err(&pdev->dev, "failed to parse audio-routing: %d\n", ret);
> + goto asrc_fail;
Hmm...audio-routing is a required property in DT binding doc.
So you might need to update that too.
^ permalink raw reply
* [PATCH kernel v2] powerpc/powernv/ioda: Return correct error if TCE level allocation failed
From: Alexey Kardashevskiy @ 2020-06-17 0:38 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Alexey Kardashevskiy
The iommu_table_ops::xchg_no_kill() callback updates TCE. It is quite
possible that not entire table is allocated if it is huge and multilevel
so xchg may also allocate subtables. If failed, it returns H_HARDWARE
for failed allocation and H_TOO_HARD if it needs it but cannot do because
the alloc parameter is "false" (set when called with MMU=off to force
retry with MMU=on).
The problem is that having separate errors only matters in real mode
(MMU=off) but the only caller with alloc="false" does not check the exact
error code and simply returns H_TOO_HARD; and for every other mode
alloc is "true". Also, the function is also called from the ioctl()
handler of the VFIO SPAPR TCE IOMMU subdriver which does not expect
hypervisor error codes (H_xxx) and will expose them to the userspace.
This converts wrong error codes to -ENOMEM.
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
Changes:
v2:
* return -ENOMEM vs. -1
---
arch/powerpc/platforms/powernv/pci-ioda-tce.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/platforms/powernv/pci-ioda-tce.c b/arch/powerpc/platforms/powernv/pci-ioda-tce.c
index f923359d8afc..5218f5da2737 100644
--- a/arch/powerpc/platforms/powernv/pci-ioda-tce.c
+++ b/arch/powerpc/platforms/powernv/pci-ioda-tce.c
@@ -166,7 +166,7 @@ int pnv_tce_xchg(struct iommu_table *tbl, long index,
if (!ptce) {
ptce = pnv_tce(tbl, false, idx, alloc);
if (!ptce)
- return alloc ? H_HARDWARE : H_TOO_HARD;
+ return -ENOMEM;
}
if (newtce & TCE_PCI_WRITE)
--
2.17.1
^ permalink raw reply related
* Re: [PATCH] scsi: target/sbp: remove firewire SBP target driver
From: Finn Thain @ 2020-06-17 2:07 UTC (permalink / raw)
To: Bart Van Assche
Cc: Martin K . Petersen, linux-scsi, Chuhong Yuan, linux-kernel,
Nicholas Bellinger, target-devel, Chris Boot, linux1394-devel,
linuxppc-dev, Stefan Richter
In-Reply-To: <8cbab988-fba7-8e27-7faf-9f7aa36ca235@acm.org>
On Tue, 16 Jun 2020, Bart Van Assche wrote:
>
> As far as I know the sbp driver only has had one user ever and that user
> is no longer user the sbp driver.
So, you estimate the userbase at zero. Can you give a confidence level?
Actual measurement is hard because when end users encounter breakage, they
look for quick workarounds before they undertake post mortem, log
collection, bug reporting, mailing list discussions, analysis etc.
> So why to keep it in the kernel tree?
Answer: for the same reason it was added to the tree.
Here's a different question: "Why remove it from the kernel tree?"
If maintaining this code is a burden, is it not the kind of tax that all
developers/users pay to all developers/users? Does this driver impose an
unreasonably high burden for some reason?
The growth of a maintenance burden in general has lead to the invention of
design patterns and tooling to minize it. So a good argument for removal
would describe the nature of the problem, because some driver deficiencies
can be fixed automatically, and some tooling deficiencies can compound an
otherwise insignificant or common driver deficiency.
There are spin-off benefits from legacy code besides process improvements.
Building and testing this sort of code has regularly revealed erroneous
corner cases in commits elsewhere like API changes and refactoring.
Also, legacy code is used by new developers get experience in code
modernization. And it provides more training material for neural networks
that need to be taught to recognize patches that raise quality.
Ten or twenty years ago, I doubt that anyone predicted these (and other)
spin-off benefits. If we can't predict the benefit, how will we project
the cost, and use that to justify deletion?
Please see also,
http://www.mac.linux-m68k.org/docs/obsolete.php
^ permalink raw reply
* Re: [PATCH] scsi: target/sbp: remove firewire SBP target driver
From: Martin K. Petersen @ 2020-06-17 2:35 UTC (permalink / raw)
To: Finn Thain
Cc: Chris Boot, Bart Van Assche, linux-scsi, Chuhong Yuan,
linux-kernel, Nicholas Bellinger, target-devel,
Martin K . Petersen, linux1394-devel, linuxppc-dev,
Stefan Richter
In-Reply-To: <alpine.LNX.2.22.394.2006140934520.15@nippy.intranet>
Finn,
> I haven't used this driver for a long time, but I still own PowerMacs
> with firewire, and I know I'm not the only one.
I also have old 1394 hardware kicking around in the basement. But having
worked with FireWire shared storage targets in the past, I have zero
desire to ever touch any of that again.
I could understand an objection if we were to entertain removing
sbp2. But really, how many people are setting up FireWire targets?
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply
* Re: [PATCH] scsi: target/sbp: remove firewire SBP target driver
From: Martin K. Petersen @ 2020-06-17 3:09 UTC (permalink / raw)
To: Chris Boot
Cc: Bart Van Assche, linux-scsi@vger.kernel.org, Johannes Thumshirn,
Chuhong Yuan, linux-kernel@vger.kernel.org, Finn Thain,
James Bottomley, target-devel@vger.kernel.org, Nicholas Bellinger,
Martin K . Petersen, linux1394-devel@lists.sourceforge.net,
linuxppc-dev@lists.ozlabs.org, Stefan Richter
In-Reply-To: <5e512185-45d1-61eb-9bec-91e9f9d53ea3@boo.tc>
Chris,
> I don't especially want it to be gone, nor can I be sure there are no
> users of what is as far as I can tell a working piece of code. I can
> tell you that I never hear about it (other than the odd patch),
> whereas I do get emails out of the blue for some of my other (much
> smaller) stuff which clearly has users. I'd be just as happy for this
> to be orphaned or for nothing to happen to it.
>
> Honestly, I am totally ambivalent as to what happens to this code.
> Martin, however, clearly cares enough to have asked me to supply a
> patch to remove it.
I love it when people take ownership of old drivers. I think it's
wonderful that Finn and others are on the ball when it comes to the 5380
family. I don't care how old things are as long as they are being
actively used.
I am also very happy to keep things in the tree as long as there is a
healthy community around them. However, keeping code around is not
free. Core interfaces change frequently. Nobody enjoys having to tweak
host templates for 50 devices they have never even heard about. Also, we
now live in a reality where there is a constant barrage of build bots
and code analyzers sending mail. So the effective cost of keeping code
around in the tree is going up. I get a substantial amount of code
analysis mail about drivers nobody have touched in a decade or more.
Consequently, I am much more inclined to remove drivers than I have been
in the past. But I am also very happy to bring them back if somebody
uses them or - even better - are willing to step up and maintain them.
I don't particularly like the notion of a driver being orphaned because
all that really means is that the driver transitions from being (at
least partially) somebody else's problem to being mine and mine alone.
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply
* Re: [PATCH 2/2] ASoC: fsl-asoc-card: Add MQS support
From: Shengjiu Wang @ 2020-06-17 3:31 UTC (permalink / raw)
To: Nicolin Chen
Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Linux-ALSA, Timur Tabi, Xiubo Li, Fabio Estevam, Shengjiu Wang,
Liam Girdwood, Takashi Iwai, Rob Herring, Mark Brown,
linuxppc-dev, linux-kernel
In-Reply-To: <20200617004845.GB19896@Asurada-Nvidia>
On Wed, Jun 17, 2020 at 8:50 AM Nicolin Chen <nicoleotsuka@gmail.com> wrote:
>
> On Tue, Jun 16, 2020 at 03:30:37PM +0800, Shengjiu Wang wrote:
> > The MQS codec isn't an i2c device, so add a new platform device for it.
> >
> > MQS only support playback, so add a new audio map.
> >
> > Add there maybe "model" property or no "audio-routing" property insertions
>
> "Add" => "And"
>
> > devicetree, so add some enhancement for these two property.
> >
> > Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
> > ---
> > sound/soc/fsl/fsl-asoc-card.c | 70 ++++++++++++++++++++++++++---------
> > 1 file changed, 52 insertions(+), 18 deletions(-)
> >
> > diff --git a/sound/soc/fsl/fsl-asoc-card.c b/sound/soc/fsl/fsl-asoc-card.c
> > index 00be73900888..2ac8cb9ddd10 100644
> > --- a/sound/soc/fsl/fsl-asoc-card.c
> > +++ b/sound/soc/fsl/fsl-asoc-card.c
>
> > @@ -482,6 +489,7 @@ static int fsl_asoc_card_probe(struct platform_device *pdev)
> > {
> > struct device_node *cpu_np, *codec_np, *asrc_np;
> > struct device_node *np = pdev->dev.of_node;
> > + struct platform_device *codec_pdev = NULL; /* used for non i2c device*/
>
> Having both codec_pdev and codec_dev duplicates things. Actually
> only a couple of places really need "codec_dev" -- most of them
> need codec_dev->dev pointer instead. So we could have a cleanup:
>
> - struct i2c_client *codec_dev;
> + struct device *codec_dev = NULL;
>
> > @@ -512,10 +520,13 @@ static int fsl_asoc_card_probe(struct platform_device *pdev)
> > }
> >
> > codec_np = of_parse_phandle(np, "audio-codec", 0);
> > - if (codec_np)
> > + if (codec_np) {
> > codec_dev = of_find_i2c_device_by_node(codec_np);
> > - else
> > + if (!codec_dev)
> > + codec_pdev = of_find_device_by_node(codec_np);
> > + } else {
> > codec_dev = NULL;
> > + }
>
> Here can have something like (feel free to simplify):
>
> if (codec_np) {
> struct platform_device *codec_pdev;
> struct i2c_client *codec_i2c;
>
> codec_i2c = of_find_i2c_device_by_node(codec_np);
> if (codec_i2c)
> codec_dev = &codec_i2c->dev;
>
> if (!codec_dev) {
> codec_pdev = of_find_device_by_node(codec_np);
> codec_dev = &codec_pdev->dev;
> }
> }
> > asrc_np = of_parse_phandle(np, "audio-asrc", 0);
> > if (asrc_np)
> > @@ -525,6 +536,13 @@ static int fsl_asoc_card_probe(struct platform_device *pdev)
> > if (codec_dev) {
> > struct clk *codec_clk = clk_get(&codec_dev->dev, NULL);
>
> Then here:
>
> - struct clk *codec_clk = clk_get(&codec_dev->dev, NULL);
> + struct clk *codec_clk = clk_get(codec_dev, NULL);
>
> > @@ -538,6 +556,11 @@ static int fsl_asoc_card_probe(struct platform_device *pdev)
> > /* Assign a default DAI format, and allow each card to overwrite it */
> > priv->dai_fmt = DAI_FMT_BASE;
> >
> > + memcpy(priv->dai_link, fsl_asoc_card_dai,
> > + sizeof(struct snd_soc_dai_link) * ARRAY_SIZE(priv->dai_link));
>
> > @@ -573,13 +596,25 @@ static int fsl_asoc_card_probe(struct platform_device *pdev)
> > codec_dai_name = "ac97-hifi";
> > priv->card.set_bias_level = NULL;
> > priv->dai_fmt = SND_SOC_DAIFMT_AC97;
> > + priv->card.dapm_routes = audio_map_ac97;
> > + priv->card.num_dapm_routes = ARRAY_SIZE(audio_map_ac97);
> > + } else if (of_device_is_compatible(np, "fsl,imx-audio-mqs")) {
> > + codec_dai_name = "fsl-mqs-dai";
> > + priv->card.set_bias_level = NULL;
> > + priv->dai_fmt = SND_SOC_DAIFMT_LEFT_J |
> > + SND_SOC_DAIFMT_CBS_CFS |
> > + SND_SOC_DAIFMT_NB_NF;
> > + priv->dai_link[1].dpcm_playback = 1;
> > + priv->dai_link[2].dpcm_playback = 1;
>
> dpcm_playback = 1? That's the default value in fsl_asoc_card_dai.
ah, should be dpcm_capture = 0.
>
> > @@ -601,19 +636,18 @@ static int fsl_asoc_card_probe(struct platform_device *pdev)
> > priv->cpu_priv.sysclk_id[0] = FSL_SAI_CLK_MAST1;
> > }
> >
> > - snprintf(priv->name, sizeof(priv->name), "%s-audio",
> > - fsl_asoc_card_is_ac97(priv) ? "ac97" :
> > - codec_dev->name);
> > -
> > /* Initialize sound card */
> > priv->pdev = pdev;
> > priv->card.dev = &pdev->dev;
> > - priv->card.name = priv->name;
> > + ret = snd_soc_of_parse_card_name(&priv->card, "model");
> > + if (ret) {
> > + snprintf(priv->name, sizeof(priv->name), "%s-audio",
> > + fsl_asoc_card_is_ac97(priv) ? "ac97" :
> > + (codec_dev ? codec_dev->name : codec_pdev->name));
>
> We can just use dev_name() if codec_dev is struct device *
> Or having a codec_dev_name to cache codec_pdev/i2c->name.
>
> > - ret = snd_soc_of_parse_audio_routing(&priv->card, "audio-routing");
> > - if (ret) {
> > - dev_err(&pdev->dev, "failed to parse audio-routing: %d\n", ret);
> > - goto asrc_fail;
> > + if (of_property_read_bool(np, "audio-routing")) {
> > + ret = snd_soc_of_parse_audio_routing(&priv->card, "audio-routing");
> > + if (ret) {
> > + dev_err(&pdev->dev, "failed to parse audio-routing: %d\n", ret);
> > + goto asrc_fail;
>
> Hmm...audio-routing is a required property in DT binding doc.
> So you might need to update that too.
will update them in v2.
best regards
wang shengjiu
^ permalink raw reply
* Re: [PATCH 2/2] ASoC: fsl_spdif: Add support for imx6sx platform
From: Shengjiu Wang @ 2020-06-17 3:43 UTC (permalink / raw)
To: Nicolin Chen
Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Linux-ALSA, Timur Tabi, Xiubo Li, Fabio Estevam, Shengjiu Wang,
Liam Girdwood, Takashi Iwai, Rob Herring, Mark Brown,
linuxppc-dev, linux-kernel
In-Reply-To: <20200616232810.GA19896@Asurada-Nvidia>
On Wed, Jun 17, 2020 at 7:30 AM Nicolin Chen <nicoleotsuka@gmail.com> wrote:
>
> On Tue, Jun 16, 2020 at 02:42:41PM +0800, Shengjiu Wang wrote:
> > The one difference on imx6sx platform is that the root clock
> > is shared with ASRC module, so we add a new flags "ind_root_clk"
> > which means the root clock is independent, then we will not
> > do the clk_set_rate and clk_round_rate to avoid impact ASRC
> > module usage.
> >
> > As add a new flags, we include the soc specific data struct.
> >
> > Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
> > ---
> > sound/soc/fsl/fsl_spdif.c | 43 +++++++++++++++++++++++++++++++++++----
> > 1 file changed, 39 insertions(+), 4 deletions(-)
> >
> > diff --git a/sound/soc/fsl/fsl_spdif.c b/sound/soc/fsl/fsl_spdif.c
> > index 1b2e516f9162..00e06803d32f 100644
> > --- a/sound/soc/fsl/fsl_spdif.c
> > +++ b/sound/soc/fsl/fsl_spdif.c
> > @@ -42,6 +42,17 @@ static u8 srpc_dpll_locked[] = { 0x0, 0x1, 0x2, 0x3, 0x4, 0xa, 0xb };
> >
> > #define DEFAULT_RXCLK_SRC 1
> >
> > +/**
> > + * struct fsl_spdif_soc_data: soc specific data
> > + *
> > + * @imx: for imx platform
> > + * @ind_root_clk: flag for round clk rate
> > + */
> > +struct fsl_spdif_soc_data {
> > + bool imx;
> > + bool ind_root_clk;
>
> "ind" doesn't look very straightforward; maybe "shared_root_clock"?
>
> And for its comments:
> * @shared_root_clock: flag of sharing a clock source with others;
> * so the driver shouldn't set root clock rate
>
> > +};
> > +
> > /*
> > * SPDIF control structure
> > * Defines channel status, subcode and Q sub
> > @@ -93,6 +104,7 @@ struct fsl_spdif_priv {
> > struct snd_soc_dai_driver cpu_dai_drv;
> > struct platform_device *pdev;
> > struct regmap *regmap;
> > + const struct fsl_spdif_soc_data *soc;
>
> Looks better if we move it to the top of the list :)
>
> > @@ -421,7 +448,7 @@ static int spdif_set_sample_rate(struct snd_pcm_substream *substream,
> > sysclk_df = spdif_priv->sysclk_df[rate];
> >
> > /* Don't mess up the clocks from other modules */
> > - if (clk != STC_TXCLK_SPDIF_ROOT)
> > + if (clk != STC_TXCLK_SPDIF_ROOT || !spdif_priv->soc->ind_root_clk)
> > goto clk_set_bypass;
> >
> > /* The S/PDIF block needs a clock of 64 * fs * txclk_df */
> > @@ -1186,7 +1213,8 @@ static int fsl_spdif_probe_txclk(struct fsl_spdif_priv *spdif_priv,
> > continue;
> >
> > ret = fsl_spdif_txclk_caldiv(spdif_priv, clk, savesub, index,
> > - i == STC_TXCLK_SPDIF_ROOT);
> > + i == STC_TXCLK_SPDIF_ROOT &&
> > + spdif_priv->soc->ind_root_clk);
>
> Having more than one place that checks the condition, we can add:
>
> /* Check if clk is a root clock that does not share clock source with others */
> static inline bool fsl_spdif_can_set_clk_rate(struct fsl_spdif_priv *spdif, int clk)
> {
> return (clk == STC_TXCLK_SPDIF_ROOT) && !spdif->soc->shared_root_clock;
> }
will update them in v2
best regards
wang shengjiu
^ permalink raw reply
* Re: [PATCH] powerpc/8xx: use pmd_off() to access a PMD entry in pte_update()
From: Mike Rapoport @ 2020-06-17 4:16 UTC (permalink / raw)
To: Michael Ellerman
Cc: Christophe Leroy, linux-kernel, Mike Rapoport, linux-mm,
Andrew Morton, linuxppc-dev
In-Reply-To: <87o8piegvt.fsf@mpe.ellerman.id.au>
On Wed, Jun 17, 2020 at 09:21:42AM +1000, Michael Ellerman wrote:
> Andrew Morton <akpm@linux-foundation.org> writes:
> > On Mon, 15 Jun 2020 12:22:29 +0300 Mike Rapoport <rppt@kernel.org> wrote:
> >
> >> From: Mike Rapoport <rppt@linux.ibm.com>
> >>
> >> The pte_update() implementation for PPC_8xx unfolds page table from the PGD
> >> level to access a PMD entry. Since 8xx has only 2-level page table this can
> >> be simplified with pmd_off() shortcut.
> >>
> >> Replace explicit unfolding with pmd_off() and drop defines of pgd_index()
> >> and pgd_offset() that are no longer needed.
> >>
> >> Signed-off-by: Mike Rapoport <rppt@linux.ibm.com>
> >> ---
> >>
> >> I think it's powerpc material, but I won't mind if Andrew picks it up :)
> >
> > Via the powerpc tree would be better, please.
>
> I'll take it into next for v5.9, unless there's a reason it needs to go
> into v5.8.
I consider it a fixup for 5.8 merge window conflicts. Besides, mering it
now may avoid new conflicts in 5.9 ;-)
> cheers
--
Sincerely yours,
Mike.
^ permalink raw reply
* Re: [PATCH] scsi: target/sbp: remove firewire SBP target driver
From: Finn Thain @ 2020-06-17 4:21 UTC (permalink / raw)
To: Martin K. Petersen
Cc: Chris Boot, Bart Van Assche, linux-scsi, Chuhong Yuan,
linux-kernel, Nicholas Bellinger, target-devel, linux1394-devel,
linuxppc-dev, Stefan Richter
In-Reply-To: <yq18sgml9ds.fsf@ca-mkp.ca.oracle.com>
On Tue, 16 Jun 2020, Martin K. Petersen wrote:
> > I haven't used this driver for a long time, but I still own PowerMacs
> > with firewire, and I know I'm not the only one.
>
I need to correct what I wrote above. I recall that years ago, when I
needed to share storage from my Linux box to my PowerBook pismo, I used
ethernet and the globalSAN iSCSI initiator for Mac OS X (which is no
longer freely available AFAICS). When I said that I had used the SBP
target driver before, I misremembered that iSCSI target setup. I've
actually never used the SBP target driver.
> I also have old 1394 hardware kicking around in the basement. But having
> worked with FireWire shared storage targets in the past, I have zero
> desire to ever touch any of that again.
>
> I could understand an objection if we were to entertain removing sbp2.
> But really, how many people are setting up FireWire targets?
>
It's a good question. I don't know the answer.
I have successfully used the Linux sbp2 host driver in the past, and will
probably need to use it again. Likewise, I can see myself using the sbp
target driver in the future because that might interoperate with MacOS 9,
and might provide a bootable device to the PowerMac ROM. iSCSI cannot do
those things.
^ permalink raw reply
* [PATCH v2 1/2] ASoC: bindings: fsl_spdif: Add new compatible string for imx6sx
From: Shengjiu Wang @ 2020-06-17 4:30 UTC (permalink / raw)
To: timur, nicoleotsuka, Xiubo.Lee, festevam, broonie, perex, tiwai,
alsa-devel, lgirdwood, robh+dt, devicetree
Cc: linuxppc-dev, linux-kernel
Add new compatible string "fsl,imx6sx-spdif" in the binding document.
And add compatible string "fsl,vf610-spdif" which was missed before.
Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
---
Documentation/devicetree/bindings/sound/fsl,spdif.txt | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/sound/fsl,spdif.txt b/Documentation/devicetree/bindings/sound/fsl,spdif.txt
index 8b324f82a782..e1365b0ee1e9 100644
--- a/Documentation/devicetree/bindings/sound/fsl,spdif.txt
+++ b/Documentation/devicetree/bindings/sound/fsl,spdif.txt
@@ -6,7 +6,11 @@ a fibre cable.
Required properties:
- - compatible : Compatible list, must contain "fsl,imx35-spdif".
+ - compatible : Compatible list, should contain one of the following
+ compatibles:
+ "fsl,imx35-spdif",
+ "fsl,vf610-spdif",
+ "fsl,imx6sx-spdif",
- reg : Offset and length of the register set for the device.
--
2.21.0
^ permalink raw reply related
* [PATCH v2 2/2] ASoC: fsl_spdif: Add support for imx6sx platform
From: Shengjiu Wang @ 2020-06-17 4:30 UTC (permalink / raw)
To: timur, nicoleotsuka, Xiubo.Lee, festevam, broonie, perex, tiwai,
alsa-devel, lgirdwood, robh+dt, devicetree
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <feda3bb02296455d43aeebb7575918d9b28e1a3f.1592368322.git.shengjiu.wang@nxp.com>
The one difference on imx6sx platform is that the root clock
is shared with ASRC module, so we add a new flags
"shared_root_clock" which means the root clock is independent,
then we will not do the clk_set_rate and clk_round_rate to avoid
impact ASRC module usage.
As add a new flags, we include the soc specific data struct.
Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
---
changes in v2
- use shared_root_clk instead ind_root_clk.
- add fsl_spdif_can_set_clk_rate function.
sound/soc/fsl/fsl_spdif.c | 50 +++++++++++++++++++++++++++++++++++----
1 file changed, 46 insertions(+), 4 deletions(-)
diff --git a/sound/soc/fsl/fsl_spdif.c b/sound/soc/fsl/fsl_spdif.c
index 1b2e516f9162..8dc1959d0463 100644
--- a/sound/soc/fsl/fsl_spdif.c
+++ b/sound/soc/fsl/fsl_spdif.c
@@ -42,6 +42,18 @@ static u8 srpc_dpll_locked[] = { 0x0, 0x1, 0x2, 0x3, 0x4, 0xa, 0xb };
#define DEFAULT_RXCLK_SRC 1
+/**
+ * struct fsl_spdif_soc_data: soc specific data
+ *
+ * @imx: for imx platform
+ * @shared_root_clock: flag of sharing a clock source with others;
+ * so the driver shouldn't set root clock rate
+ */
+struct fsl_spdif_soc_data {
+ bool imx;
+ bool shared_root_clock;
+};
+
/*
* SPDIF control structure
* Defines channel status, subcode and Q sub
@@ -89,6 +101,7 @@ struct spdif_mixer_control {
* @dma_params_rx: DMA parameters for receive channel
*/
struct fsl_spdif_priv {
+ const struct fsl_spdif_soc_data *soc;
struct spdif_mixer_control fsl_spdif_control;
struct snd_soc_dai_driver cpu_dai_drv;
struct platform_device *pdev;
@@ -110,6 +123,28 @@ struct fsl_spdif_priv {
u32 regcache_srpc;
};
+static struct fsl_spdif_soc_data fsl_spdif_vf610 = {
+ .imx = false,
+ .shared_root_clock = false,
+};
+
+static struct fsl_spdif_soc_data fsl_spdif_imx35 = {
+ .imx = true,
+ .shared_root_clock = false,
+};
+
+static struct fsl_spdif_soc_data fsl_spdif_imx6sx = {
+ .imx = true,
+ .shared_root_clock = true,
+};
+
+/* Check if clk is a root clock that does not share clock source with others */
+static inline bool fsl_spdif_can_set_clk_rate(struct fsl_spdif_priv *spdif,
+ int clk)
+{
+ return (clk == STC_TXCLK_SPDIF_ROOT) && !spdif->soc->shared_root_clock;
+}
+
/* DPLL locked and lock loss interrupt handler */
static void spdif_irq_dpll_lock(struct fsl_spdif_priv *spdif_priv)
{
@@ -421,7 +456,7 @@ static int spdif_set_sample_rate(struct snd_pcm_substream *substream,
sysclk_df = spdif_priv->sysclk_df[rate];
/* Don't mess up the clocks from other modules */
- if (clk != STC_TXCLK_SPDIF_ROOT)
+ if (!fsl_spdif_can_set_clk_rate(spdif_priv, clk))
goto clk_set_bypass;
/* The S/PDIF block needs a clock of 64 * fs * txclk_df */
@@ -1186,7 +1221,7 @@ static int fsl_spdif_probe_txclk(struct fsl_spdif_priv *spdif_priv,
continue;
ret = fsl_spdif_txclk_caldiv(spdif_priv, clk, savesub, index,
- i == STC_TXCLK_SPDIF_ROOT);
+ fsl_spdif_can_set_clk_rate(spdif_priv, i));
if (savesub == ret)
continue;
@@ -1230,6 +1265,12 @@ static int fsl_spdif_probe(struct platform_device *pdev)
spdif_priv->pdev = pdev;
+ spdif_priv->soc = of_device_get_match_data(&pdev->dev);
+ if (!spdif_priv->soc) {
+ dev_err(&pdev->dev, "failed to get soc data\n");
+ return -ENODEV;
+ }
+
/* Initialize this copy of the CPU DAI driver structure */
memcpy(&spdif_priv->cpu_dai_drv, &fsl_spdif_dai, sizeof(fsl_spdif_dai));
spdif_priv->cpu_dai_drv.name = dev_name(&pdev->dev);
@@ -1359,8 +1400,9 @@ static const struct dev_pm_ops fsl_spdif_pm = {
};
static const struct of_device_id fsl_spdif_dt_ids[] = {
- { .compatible = "fsl,imx35-spdif", },
- { .compatible = "fsl,vf610-spdif", },
+ { .compatible = "fsl,imx35-spdif", .data = &fsl_spdif_imx35, },
+ { .compatible = "fsl,vf610-spdif", .data = &fsl_spdif_vf610, },
+ { .compatible = "fsl,imx6sx-spdif", .data = &fsl_spdif_imx6sx, },
{}
};
MODULE_DEVICE_TABLE(of, fsl_spdif_dt_ids);
--
2.21.0
^ permalink raw reply related
* [PATCH v2 1/2] ASoC: bindings: fsl-asoc-card: Add compatible string for MQS
From: Shengjiu Wang @ 2020-06-17 4:48 UTC (permalink / raw)
To: timur, nicoleotsuka, Xiubo.Lee, festevam, broonie, perex, tiwai,
alsa-devel, lgirdwood, robh+dt, devicetree
Cc: linuxppc-dev, linux-kernel
Add compatible string "fsl,imx-audio-mqs" for MQS, and move
"audio-routing" property to be optional for MQS doesn't need
such property.
Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
---
changes in v2
- Move "audio-routing" to optional.
.../devicetree/bindings/sound/fsl-asoc-card.txt | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/Documentation/devicetree/bindings/sound/fsl-asoc-card.txt b/Documentation/devicetree/bindings/sound/fsl-asoc-card.txt
index c60a5732d29c..ca9a3a43adfd 100644
--- a/Documentation/devicetree/bindings/sound/fsl-asoc-card.txt
+++ b/Documentation/devicetree/bindings/sound/fsl-asoc-card.txt
@@ -34,6 +34,8 @@ The compatible list for this generic sound card currently:
"fsl,imx-audio-wm8960"
+ "fsl,imx-audio-mqs"
+
Required properties:
- compatible : Contains one of entries in the compatible list.
@@ -44,6 +46,11 @@ Required properties:
- audio-codec : The phandle of an audio codec
+Optional properties:
+
+ - audio-asrc : The phandle of ASRC. It can be absent if there's no
+ need to add ASRC support via DPCM.
+
- audio-routing : A list of the connections between audio components.
Each entry is a pair of strings, the first being the
connection's sink, the second being the connection's
@@ -60,11 +67,6 @@ Required properties:
coexisting in order to support the old bindings
of wm8962 and sgtl5000.
-Optional properties:
-
- - audio-asrc : The phandle of ASRC. It can be absent if there's no
- need to add ASRC support via DPCM.
-
Optional unless SSI is selected as a CPU DAI:
- mux-int-port : The internal port of the i.MX audio muxer (AUDMUX)
--
2.21.0
^ permalink raw reply related
* [PATCH v2 2/2] ASoC: fsl-asoc-card: Add MQS support
From: Shengjiu Wang @ 2020-06-17 4:48 UTC (permalink / raw)
To: timur, nicoleotsuka, Xiubo.Lee, festevam, broonie, perex, tiwai,
alsa-devel, lgirdwood, robh+dt, devicetree
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <2185a3ec866bc59f82d93b73d1a732a896fd8f48.1592369271.git.shengjiu.wang@nxp.com>
The MQS codec isn't an i2c device, so use of_find_device_by_node
to get platform device pointer.
Because MQS only support playback, then add a new audio map.
And there maybe "model" property or no "audio-routing" property in
devicetree, so add some enhancement for these two property.
Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
---
changes in v2
- update according Nicolin's comments.
sound/soc/fsl/fsl-asoc-card.c | 78 +++++++++++++++++++++++++----------
1 file changed, 57 insertions(+), 21 deletions(-)
diff --git a/sound/soc/fsl/fsl-asoc-card.c b/sound/soc/fsl/fsl-asoc-card.c
index 00be73900888..d0543a53764e 100644
--- a/sound/soc/fsl/fsl-asoc-card.c
+++ b/sound/soc/fsl/fsl-asoc-card.c
@@ -119,6 +119,13 @@ static const struct snd_soc_dapm_route audio_map_ac97[] = {
{"ASRC-Capture", NULL, "AC97 Capture"},
};
+static const struct snd_soc_dapm_route audio_map_tx[] = {
+ /* 1st half -- Normal DAPM routes */
+ {"Playback", NULL, "CPU-Playback"},
+ /* 2nd half -- ASRC DAPM routes */
+ {"CPU-Playback", NULL, "ASRC-Playback"},
+};
+
/* Add all possible widgets into here without being redundant */
static const struct snd_soc_dapm_widget fsl_asoc_card_dapm_widgets[] = {
SND_SOC_DAPM_LINE("Line Out Jack", NULL),
@@ -485,8 +492,9 @@ static int fsl_asoc_card_probe(struct platform_device *pdev)
struct platform_device *asrc_pdev = NULL;
struct platform_device *cpu_pdev;
struct fsl_asoc_card_priv *priv;
- struct i2c_client *codec_dev;
+ struct device *codec_dev = NULL;
const char *codec_dai_name;
+ const char *codec_dev_name;
u32 width;
int ret;
@@ -512,10 +520,23 @@ static int fsl_asoc_card_probe(struct platform_device *pdev)
}
codec_np = of_parse_phandle(np, "audio-codec", 0);
- if (codec_np)
- codec_dev = of_find_i2c_device_by_node(codec_np);
- else
- codec_dev = NULL;
+ if (codec_np) {
+ struct platform_device *codec_pdev;
+ struct i2c_client *codec_i2c;
+
+ codec_i2c = of_find_i2c_device_by_node(codec_np);
+ if (codec_i2c) {
+ codec_dev = &codec_i2c->dev;
+ codec_dev_name = codec_i2c->name;
+ }
+ if (!codec_dev) {
+ codec_pdev = of_find_device_by_node(codec_np);
+ if (codec_pdev) {
+ codec_dev = &codec_pdev->dev;
+ codec_dev_name = codec_pdev->name;
+ }
+ }
+ }
asrc_np = of_parse_phandle(np, "audio-asrc", 0);
if (asrc_np)
@@ -523,7 +544,7 @@ static int fsl_asoc_card_probe(struct platform_device *pdev)
/* Get the MCLK rate only, and leave it controlled by CODEC drivers */
if (codec_dev) {
- struct clk *codec_clk = clk_get(&codec_dev->dev, NULL);
+ struct clk *codec_clk = clk_get(codec_dev, NULL);
if (!IS_ERR(codec_clk)) {
priv->codec_priv.mclk_freq = clk_get_rate(codec_clk);
@@ -538,6 +559,11 @@ static int fsl_asoc_card_probe(struct platform_device *pdev)
/* Assign a default DAI format, and allow each card to overwrite it */
priv->dai_fmt = DAI_FMT_BASE;
+ memcpy(priv->dai_link, fsl_asoc_card_dai,
+ sizeof(struct snd_soc_dai_link) * ARRAY_SIZE(priv->dai_link));
+
+ priv->card.dapm_routes = audio_map;
+ priv->card.num_dapm_routes = ARRAY_SIZE(audio_map);
/* Diversify the card configurations */
if (of_device_is_compatible(np, "fsl,imx-audio-cs42888")) {
codec_dai_name = "cs42888";
@@ -573,6 +599,18 @@ static int fsl_asoc_card_probe(struct platform_device *pdev)
codec_dai_name = "ac97-hifi";
priv->card.set_bias_level = NULL;
priv->dai_fmt = SND_SOC_DAIFMT_AC97;
+ priv->card.dapm_routes = audio_map_ac97;
+ priv->card.num_dapm_routes = ARRAY_SIZE(audio_map_ac97);
+ } else if (of_device_is_compatible(np, "fsl,imx-audio-mqs")) {
+ codec_dai_name = "fsl-mqs-dai";
+ priv->card.set_bias_level = NULL;
+ priv->dai_fmt = SND_SOC_DAIFMT_LEFT_J |
+ SND_SOC_DAIFMT_CBS_CFS |
+ SND_SOC_DAIFMT_NB_NF;
+ priv->dai_link[1].dpcm_capture = 0;
+ priv->dai_link[2].dpcm_capture = 0;
+ priv->card.dapm_routes = audio_map_tx;
+ priv->card.num_dapm_routes = ARRAY_SIZE(audio_map_tx);
} else {
dev_err(&pdev->dev, "unknown Device Tree compatible\n");
ret = -EINVAL;
@@ -601,19 +639,17 @@ static int fsl_asoc_card_probe(struct platform_device *pdev)
priv->cpu_priv.sysclk_id[0] = FSL_SAI_CLK_MAST1;
}
- snprintf(priv->name, sizeof(priv->name), "%s-audio",
- fsl_asoc_card_is_ac97(priv) ? "ac97" :
- codec_dev->name);
-
/* Initialize sound card */
priv->pdev = pdev;
priv->card.dev = &pdev->dev;
- priv->card.name = priv->name;
+ ret = snd_soc_of_parse_card_name(&priv->card, "model");
+ if (ret) {
+ snprintf(priv->name, sizeof(priv->name), "%s-audio",
+ fsl_asoc_card_is_ac97(priv) ? "ac97" : codec_dev_name);
+ priv->card.name = priv->name;
+ }
priv->card.dai_link = priv->dai_link;
- priv->card.dapm_routes = fsl_asoc_card_is_ac97(priv) ?
- audio_map_ac97 : audio_map;
priv->card.late_probe = fsl_asoc_card_late_probe;
- priv->card.num_dapm_routes = ARRAY_SIZE(audio_map);
priv->card.dapm_widgets = fsl_asoc_card_dapm_widgets;
priv->card.num_dapm_widgets = ARRAY_SIZE(fsl_asoc_card_dapm_widgets);
@@ -621,13 +657,12 @@ static int fsl_asoc_card_probe(struct platform_device *pdev)
if (!asrc_pdev)
priv->card.num_dapm_routes /= 2;
- memcpy(priv->dai_link, fsl_asoc_card_dai,
- sizeof(struct snd_soc_dai_link) * ARRAY_SIZE(priv->dai_link));
-
- ret = snd_soc_of_parse_audio_routing(&priv->card, "audio-routing");
- if (ret) {
- dev_err(&pdev->dev, "failed to parse audio-routing: %d\n", ret);
- goto asrc_fail;
+ if (of_property_read_bool(np, "audio-routing")) {
+ ret = snd_soc_of_parse_audio_routing(&priv->card, "audio-routing");
+ if (ret) {
+ dev_err(&pdev->dev, "failed to parse audio-routing: %d\n", ret);
+ goto asrc_fail;
+ }
}
/* Normal DAI Link */
@@ -724,6 +759,7 @@ static const struct of_device_id fsl_asoc_card_dt_ids[] = {
{ .compatible = "fsl,imx-audio-sgtl5000", },
{ .compatible = "fsl,imx-audio-wm8962", },
{ .compatible = "fsl,imx-audio-wm8960", },
+ { .compatible = "fsl,imx-audio-mqs", },
{}
};
MODULE_DEVICE_TABLE(of, fsl_asoc_card_dt_ids);
--
2.21.0
^ permalink raw reply related
* Re: [PATCH v2 2/2] ASoC: fsl_spdif: Add support for imx6sx platform
From: Nicolin Chen @ 2020-06-17 6:24 UTC (permalink / raw)
To: Shengjiu Wang
Cc: devicetree, alsa-devel, timur, Xiubo.Lee, lgirdwood, linuxppc-dev,
tiwai, robh+dt, perex, broonie, festevam, linux-kernel
In-Reply-To: <53a969a83999de91f3ff2809d78335c3f0cc1ee3.1592368322.git.shengjiu.wang@nxp.com>
On Wed, Jun 17, 2020 at 12:30:17PM +0800, Shengjiu Wang wrote:
> The one difference on imx6sx platform is that the root clock
> is shared with ASRC module, so we add a new flags
> "shared_root_clock" which means the root clock is independent,
"shared" means "not independent", against "independent" ;)
> then we will not do the clk_set_rate and clk_round_rate to avoid
> impact ASRC module usage.
>
> As add a new flags, we include the soc specific data struct.
>
> Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
Can add this once fixing the remaining comments:
Reviewed-by: Nicolin Chen <nicoleotsuka@gmail.com>
> +static inline bool fsl_spdif_can_set_clk_rate(struct fsl_spdif_priv *spdif,
> + int clk)
Can actually merge into single line as kernel has 100-character
limit now, though 80-char is still preferable for a good coding
style. But I think this one wouldn't be too bad at all.
> @@ -421,7 +456,7 @@ static int spdif_set_sample_rate(struct snd_pcm_substream *substream,
> sysclk_df = spdif_priv->sysclk_df[rate];
>
> /* Don't mess up the clocks from other modules */
We can drop this comments now as it's out-of-date and the name of
the new helper function is straightforward enough.
> - if (clk != STC_TXCLK_SPDIF_ROOT)
> + if (!fsl_spdif_can_set_clk_rate(spdif_priv, clk))
> goto clk_set_bypass;
>
> /* The S/PDIF block needs a clock of 64 * fs * txclk_df */
^ permalink raw reply
* Re: powerpc/pci: [PATCH 1/1 V3] PCIE PHB reset
From: Michael Ellerman @ 2020-06-17 6:30 UTC (permalink / raw)
To: Oliver O'Halloran; +Cc: Brian King, Wen Xiong, linuxppc-dev, wenxiong
In-Reply-To: <CAOSf1CHT9A55+ZAKquikHy9Siy_k5E0ucB-qY2G7hjfVSmf7pg@mail.gmail.com>
"Oliver O'Halloran" <oohall@gmail.com> writes:
> On Tue, Jun 16, 2020 at 9:55 PM Michael Ellerman <mpe@ellerman.id.au> wrote:
>> wenxiong@linux.vnet.ibm.com writes:
>> > From: Wen Xiong <wenxiong@linux.vnet.ibm.com>
>> >
>> > Several device drivers hit EEH(Extended Error handling) when triggering
>> > kdump on Pseries PowerVM. This patch implemented a reset of the PHBs
>> > in pci general code when triggering kdump.
>>
>> Actually it's in pseries specific PCI code, and the reset is done in the
>> 2nd kernel as it boots, not when triggering the kdump.
>>
>> You're doing it as a:
>>
>> machine_postcore_initcall(pseries, pseries_phb_reset);
>>
>> But we do the EEH initialisation in:
>>
>> core_initcall_sync(eeh_init);
>>
>> Which happens first.
>>
>> So it seems to me that this should be called from pseries_eeh_init().
>
> This happens to use some of the same RTAS calls as EEH, but it's
> entirely orthogonal to it.
I don't agree. I mean it's literally calling EEH_RESET_FUNDAMENTAL etc.
Those RTAS calls are all documented in the EEH section of PAPR.
I guess you're saying it's orthogonal to the kernel handling an EEH and
doing the recovery process etc, which I can kind of see.
> Wedging the two together doesn't make any real sense IMO since this
> should be usable even with !CONFIG_EEH.
You can't turn CONFIG_EEH off for pseries or powernv.
And if you could this patch wouldn't compile because it uses EEH
constants that are behind #ifdef CONFIG_EEH.
If you could turn CONFIG_EEH off it would presumably be because you were
on a platform that didn't support EEH, in which case you wouldn't need
this code.
So IMO this is EEH code, and should be with the other EEH code and
should be behind CONFIG_EEH.
>> That would isolate the code in the right place, and allow you to use the
>> existing ibm_get_config_addr_info.
>>
>> You probably can't use pseries_eeh_get_pe_addr(), because you won't have
>> a "pe" structure yet.
>>
>> Instead you should add a helper that does the core of that logic but
>> accepts config_addr/buid as parameters, and then have your code and
>> pseries_eeh_get_pe_addr() call that.
>
> I have a patch in my next pile of eeh reworks which kills off
> pseries_eeh_get_pe_addr() entirely. It's used to implement
> eeh_ops->get_pe_addr for pseries, but the only caller of
> ->get_pe_addr() is in pseries EEH code and the powernv version is a
> stub so I was going to drop it from EEH ops and fold it into the
> caller. We could do that in this patch, but it's just going to create
> a merge conflict for you to deal with. Up to you.
That sounds like a good cleanup. I'm not concerned about conflicts
within arch/powerpc, I can fix them up.
>> > + list_for_each_entry(phb, &hose_list, list_node) {
>> > + config_addr = pseries_get_pdn_addr(phb);
>> > + if (config_addr == -1)
>> > + continue;
>> > +
>> > + ret = rtas_call(ibm_set_slot_reset, 4, 1, NULL,
>> > + config_addr, BUID_HI(phb->buid),
>> > + BUID_LO(phb->buid), EEH_RESET_FUNDAMENTAL);
>> > +
>> > + /* If fundamental-reset not supported, try hot-reset */
>> > + if (ret == -8)
>>
>> Where does -8 come from?
>
> There's a comment right there.
Yeah I guess. I was expecting it would map to some RTAS_ERROR_FOO value,
but it's just literally -8 in PAPR.
As long as there's only a single usage then I don't mind it.
> It could be better explained I suppose, but you need to read
> PAPR/LoPAPR to make sense of anything RTAS so what's it really buying
> you?
Making the code easier for new folks to understand?
cheers
^ permalink raw reply
* Re: [PATCH v2 2/2] ASoC: fsl-asoc-card: Add MQS support
From: Nicolin Chen @ 2020-06-17 6:30 UTC (permalink / raw)
To: Shengjiu Wang
Cc: devicetree, alsa-devel, timur, Xiubo.Lee, lgirdwood, linuxppc-dev,
tiwai, robh+dt, perex, broonie, festevam, linux-kernel
In-Reply-To: <918505decb7f757f12c38059c590984f28d2f3a4.1592369271.git.shengjiu.wang@nxp.com>
On Wed, Jun 17, 2020 at 12:48:25PM +0800, Shengjiu Wang wrote:
> The MQS codec isn't an i2c device, so use of_find_device_by_node
> to get platform device pointer.
>
> Because MQS only support playback, then add a new audio map.
>
> And there maybe "model" property or no "audio-routing" property in
> devicetree, so add some enhancement for these two property.
>
> Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
Reviewed-by: Nicolin Chen <nicoleotsuka@gmail.com>
^ permalink raw reply
* Re: [PATCH 1/2] selftests/powerpc: Allow choice of CI memory location in alignment_handler test
From: Alistair Popple @ 2020-06-17 6:31 UTC (permalink / raw)
To: Jordan Niethe; +Cc: Sachin Sant, linuxppc-dev
In-Reply-To: <20200520021103.19798-1-jniethe5@gmail.com>
I've tested these on Mambo and they pass so:
Tested-by: Alistair Popple <alistair@popple.id.au>
On Wednesday, 20 May 2020 12:11:02 PM AEST Jordan Niethe wrote:
> The alignment handler selftest needs cache-inhibited memory and
> currently /dev/fb0 is relied on to provided this. This prevents running
> the test on systems without /dev/fb0 (e.g., mambo). Read the commandline
> arguments for an optional path to be used instead, as well as an
> optional offset to be for mmaping this path.
>
> Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
> ---
> .../powerpc/alignment/alignment_handler.c | 63 ++++++++++++-------
> 1 file changed, 42 insertions(+), 21 deletions(-)
>
> diff --git a/tools/testing/selftests/powerpc/alignment/alignment_handler.c b/tools/testing/selftests/powerpc/alignment/alignment_handler.c
> index 0453c50c949c..eb6aba323f8b 100644
> --- a/tools/testing/selftests/powerpc/alignment/alignment_handler.c
> +++ b/tools/testing/selftests/powerpc/alignment/alignment_handler.c
> @@ -9,7 +9,17 @@
> * This selftest exercises the powerpc alignment fault handler.
> *
> * We create two sets of source and destination buffers, one in regular memory,
> - * the other cache-inhibited (we use /dev/fb0 for this).
> + * the other cache-inhibited (by default we use /dev/fb0 for this, but an
> + * alterative path for cache-inhibited memory may be provided).
> + *
> + * One way to get cache-inhibited memory is to use the "mem" kernel parameter
> + * to limit the kernel to less memory than actually exists. Addresses above
> + * the limit may still be accessed but will be treated as cache-inhibited. For
> + * example, if there is actually 4GB of memory and the parameter "mem=3GB" is
> + * used, memory from address 0xC0000000 onwards is treated as cache-inhibited.
> + * To access this region /dev/mem is used. The kernel should be configured
> + * without CONFIG_STRICT_DEVMEM. In this case use:
> + * ./alignment_handler /dev/mem 0xc0000000
> *
> * We initialise the source buffers, then use whichever set of load/store
> * instructions is under test to copy bytes from the source buffers to the
> @@ -53,6 +63,8 @@ int bufsize;
> int debug;
> int testing;
> volatile int gotsig;
> +char *cipath = "/dev/fb0";
> +long cioffset;
>
> void sighandler(int sig, siginfo_t *info, void *ctx)
> {
> @@ -195,17 +207,18 @@ int do_test(char *test_name, void (*test_func)(char *, char *))
>
> printf("\tDoing %s:\t", test_name);
>
> - fd = open("/dev/fb0", O_RDWR);
> + fd = open(cipath, O_RDWR);
> if (fd < 0) {
> printf("\n");
> - perror("Can't open /dev/fb0 now?");
> + perror("Can't open ci file now?");
> return 1;
> }
>
> - ci0 = mmap(NULL, bufsize, PROT_WRITE, MAP_SHARED,
> - fd, 0x0);
> - ci1 = mmap(NULL, bufsize, PROT_WRITE, MAP_SHARED,
> - fd, bufsize);
> + ci0 = mmap(NULL, bufsize, PROT_WRITE | PROT_READ, MAP_SHARED,
> + fd, cioffset);
> + ci1 = mmap(NULL, bufsize, PROT_WRITE | PROT_READ, MAP_SHARED,
> + fd, cioffset + bufsize);
> +
> if ((ci0 == MAP_FAILED) || (ci1 == MAP_FAILED)) {
> printf("\n");
> perror("mmap failed");
> @@ -270,11 +283,11 @@ int do_test(char *test_name, void (*test_func)(char *, char *))
> return rc;
> }
>
> -static bool can_open_fb0(void)
> +static bool can_open_cifile(void)
> {
> int fd;
>
> - fd = open("/dev/fb0", O_RDWR);
> + fd = open(cipath, O_RDWR);
> if (fd < 0)
> return false;
>
> @@ -286,7 +299,7 @@ int test_alignment_handler_vsx_206(void)
> {
> int rc = 0;
>
> - SKIP_IF(!can_open_fb0());
> + SKIP_IF(!can_open_cifile());
> SKIP_IF(!have_hwcap(PPC_FEATURE_ARCH_2_06));
>
> printf("VSX: 2.06B\n");
> @@ -304,7 +317,7 @@ int test_alignment_handler_vsx_207(void)
> {
> int rc = 0;
>
> - SKIP_IF(!can_open_fb0());
> + SKIP_IF(!can_open_cifile());
> SKIP_IF(!have_hwcap2(PPC_FEATURE2_ARCH_2_07));
>
> printf("VSX: 2.07B\n");
> @@ -320,7 +333,7 @@ int test_alignment_handler_vsx_300(void)
> {
> int rc = 0;
>
> - SKIP_IF(!can_open_fb0());
> + SKIP_IF(!can_open_cifile());
>
> SKIP_IF(!have_hwcap2(PPC_FEATURE2_ARCH_3_00));
> printf("VSX: 3.00B\n");
> @@ -352,7 +365,7 @@ int test_alignment_handler_integer(void)
> {
> int rc = 0;
>
> - SKIP_IF(!can_open_fb0());
> + SKIP_IF(!can_open_cifile());
>
> printf("Integer\n");
> LOAD_DFORM_TEST(lbz);
> @@ -408,7 +421,7 @@ int test_alignment_handler_integer_206(void)
> {
> int rc = 0;
>
> - SKIP_IF(!can_open_fb0());
> + SKIP_IF(!can_open_cifile());
> SKIP_IF(!have_hwcap(PPC_FEATURE_ARCH_2_06));
>
> printf("Integer: 2.06\n");
> @@ -423,7 +436,7 @@ int test_alignment_handler_vmx(void)
> {
> int rc = 0;
>
> - SKIP_IF(!can_open_fb0());
> + SKIP_IF(!can_open_cifile());
> SKIP_IF(!have_hwcap(PPC_FEATURE_HAS_ALTIVEC));
>
> printf("VMX\n");
> @@ -451,7 +464,7 @@ int test_alignment_handler_fp(void)
> {
> int rc = 0;
>
> - SKIP_IF(!can_open_fb0());
> + SKIP_IF(!can_open_cifile());
>
> printf("Floating point\n");
> LOAD_FLOAT_DFORM_TEST(lfd);
> @@ -479,7 +492,7 @@ int test_alignment_handler_fp_205(void)
> {
> int rc = 0;
>
> - SKIP_IF(!can_open_fb0());
> + SKIP_IF(!can_open_cifile());
> SKIP_IF(!have_hwcap(PPC_FEATURE_ARCH_2_05));
>
> printf("Floating point: 2.05\n");
> @@ -497,7 +510,7 @@ int test_alignment_handler_fp_206(void)
> {
> int rc = 0;
>
> - SKIP_IF(!can_open_fb0());
> + SKIP_IF(!can_open_cifile());
> SKIP_IF(!have_hwcap(PPC_FEATURE_ARCH_2_06));
>
> printf("Floating point: 2.06\n");
> @@ -509,11 +522,12 @@ int test_alignment_handler_fp_206(void)
>
> void usage(char *prog)
> {
> - printf("Usage: %s [options]\n", prog);
> + printf("Usage: %s [options] [path [offset]]\n", prog);
> printf(" -d Enable debug error output\n");
> printf("\n");
> - printf("This test requires a POWER8 or POWER9 CPU and a usable ");
> - printf("framebuffer at /dev/fb0.\n");
> + printf("This test requires a POWER8 or POWER9 CPU and either a ");
> + printf("usable framebuffer at /dev/fb0 or the path to usable ");
> + printf("cache-inhibited memory and optional offset to be provided\n");
> }
>
> int main(int argc, char *argv[])
> @@ -533,6 +547,13 @@ int main(int argc, char *argv[])
> exit(1);
> }
> }
> + argc -= optind;
> + argv += optind;
> +
> + if (argc > 0)
> + cipath = argv[0];
> + if (argc > 1)
> + cioffset = strtol(argv[1], 0, 0x10);
>
> bufsize = getpagesize();
>
>
^ permalink raw reply
* Re: [PATCH 2/2] selftests/powerpc: Add prefixed loads/stores to alignment_handler test
From: Alistair Popple @ 2020-06-17 6:31 UTC (permalink / raw)
To: Jordan Niethe; +Cc: sachinp, linuxppc-dev
In-Reply-To: <20200520021103.19798-2-jniethe5@gmail.com>
Tested-by: Alistair Popple <alistair@popple.id.au>
On Wednesday, 20 May 2020 12:11:03 PM AEST Jordan Niethe wrote:
> Extend the alignment handler selftest to exercise prefixed load store
> instructions. Add tests for prefixed VSX, floating point and integer
> instructions.
>
> Skip prefix tests if ISA version does not support prefixed instructions.
>
> Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
> ---
> .../powerpc/alignment/alignment_handler.c | 93 ++++++++++++++++++-
> .../selftests/powerpc/include/instructions.h | 77 +++++++++++++++
> .../testing/selftests/powerpc/include/utils.h | 5 +
> 3 files changed, 172 insertions(+), 3 deletions(-)
>
> diff --git a/tools/testing/selftests/powerpc/alignment/alignment_handler.c b/tools/testing/selftests/powerpc/alignment/alignment_handler.c
> index eb6aba323f8b..e582e68b3b5b 100644
> --- a/tools/testing/selftests/powerpc/alignment/alignment_handler.c
> +++ b/tools/testing/selftests/powerpc/alignment/alignment_handler.c
> @@ -58,6 +58,7 @@
> #include <asm/cputable.h>
>
> #include "utils.h"
> +#include "instructions.h"
>
> int bufsize;
> int debug;
> @@ -96,6 +97,17 @@ void sighandler(int sig, siginfo_t *info, void *ctx)
> } \
> rc |= do_test(#name, test_##name)
>
> +#define TESTP(name, ld_op, st_op, ld_reg, st_reg) \
> + void test_##name(char *s, char *d) \
> + { \
> + asm volatile( \
> + ld_op(ld_reg, %0, 0, 0) \
> + st_op(st_reg, %1, 0, 0) \
> + :: "r"(s), "r"(d), "r"(0) \
> + : "memory", "vs0", "vs32", "r31"); \
> + } \
> + rc |= do_test(#name, test_##name)
> +
> #define LOAD_VSX_XFORM_TEST(op) TEST(op, op, stxvd2x, XFORM, 32, 32)
> #define STORE_VSX_XFORM_TEST(op) TEST(op, lxvd2x, op, XFORM, 32, 32)
> #define LOAD_VSX_DFORM_TEST(op) TEST(op, op, stxv, DFORM, 32, 32)
> @@ -115,6 +127,17 @@ void sighandler(int sig, siginfo_t *info, void *ctx)
> #define LOAD_FLOAT_XFORM_TEST(op) TEST(op, op, stfdx, XFORM, 0, 0)
> #define STORE_FLOAT_XFORM_TEST(op) TEST(op, lfdx, op, XFORM, 0, 0)
>
> +#define LOAD_MLS_PREFIX_TEST(op) TESTP(op, op, PSTD, 31, 31)
> +#define STORE_MLS_PREFIX_TEST(op) TESTP(op, PLD, op, 31, 31)
> +
> +#define LOAD_8LS_PREFIX_TEST(op) TESTP(op, op, PSTD, 31, 31)
> +#define STORE_8LS_PREFIX_TEST(op) TESTP(op, PLD, op, 31, 31)
> +
> +#define LOAD_FLOAT_MLS_PREFIX_TEST(op) TESTP(op, op, PSTFD, 0, 0)
> +#define STORE_FLOAT_MLS_PREFIX_TEST(op) TESTP(op, PLFD, op, 0, 0)
> +
> +#define LOAD_VSX_8LS_PREFIX_TEST(op, tail) TESTP(op, op, PSTXV ## tail, 0, 32)
> +#define STORE_VSX_8LS_PREFIX_TEST(op, tail) TESTP(op, PLXV ## tail, op, 32, 0)
>
> /* FIXME: Unimplemented tests: */
> // STORE_DFORM_TEST(stq) /* FIXME: need two registers for quad */
> @@ -361,6 +384,25 @@ int test_alignment_handler_vsx_300(void)
> return rc;
> }
>
> +int test_alignment_handler_vsx_prefix(void)
> +{
> + int rc = 0;
> +
> + SKIP_IF(!can_open_cifile());
> + SKIP_IF(!have_hwcap2(PPC_FEATURE2_ARCH_3_10));
> +
> + printf("VSX: PREFIX\n");
> + LOAD_VSX_8LS_PREFIX_TEST(PLXSD, 0);
> + LOAD_VSX_8LS_PREFIX_TEST(PLXSSP, 0);
> + LOAD_VSX_8LS_PREFIX_TEST(PLXV0, 0);
> + LOAD_VSX_8LS_PREFIX_TEST(PLXV1, 1);
> + STORE_VSX_8LS_PREFIX_TEST(PSTXSD, 0);
> + STORE_VSX_8LS_PREFIX_TEST(PSTXSSP, 0);
> + STORE_VSX_8LS_PREFIX_TEST(PSTXV0, 0);
> + STORE_VSX_8LS_PREFIX_TEST(PSTXV1, 1);
> + return rc;
> +}
> +
> int test_alignment_handler_integer(void)
> {
> int rc = 0;
> @@ -432,6 +474,27 @@ int test_alignment_handler_integer_206(void)
> return rc;
> }
>
> +int test_alignment_handler_integer_prefix(void)
> +{
> + int rc = 0;
> +
> + SKIP_IF(!can_open_cifile());
> + SKIP_IF(!have_hwcap2(PPC_FEATURE2_ARCH_3_10));
> +
> + printf("Integer: PREFIX\n");
> + LOAD_MLS_PREFIX_TEST(PLBZ);
> + LOAD_MLS_PREFIX_TEST(PLHZ);
> + LOAD_MLS_PREFIX_TEST(PLHA);
> + LOAD_MLS_PREFIX_TEST(PLWZ);
> + LOAD_8LS_PREFIX_TEST(PLWA);
> + LOAD_8LS_PREFIX_TEST(PLD);
> + STORE_MLS_PREFIX_TEST(PSTB);
> + STORE_MLS_PREFIX_TEST(PSTH);
> + STORE_MLS_PREFIX_TEST(PSTW);
> + STORE_8LS_PREFIX_TEST(PSTD);
> + return rc;
> +}
> +
> int test_alignment_handler_vmx(void)
> {
> int rc = 0;
> @@ -520,14 +583,32 @@ int test_alignment_handler_fp_206(void)
> return rc;
> }
>
> +
> +int test_alignment_handler_fp_prefix(void)
> +{
> + int rc = 0;
> +
> + SKIP_IF(!can_open_cifile());
> + SKIP_IF(!have_hwcap2(PPC_FEATURE2_ARCH_3_10));
> +
> + printf("Floating point: PREFIX\n");
> + LOAD_FLOAT_DFORM_TEST(lfs);
> + LOAD_FLOAT_MLS_PREFIX_TEST(PLFS);
> + LOAD_FLOAT_MLS_PREFIX_TEST(PLFD);
> + STORE_FLOAT_MLS_PREFIX_TEST(PSTFS);
> + STORE_FLOAT_MLS_PREFIX_TEST(PSTFD);
> + return rc;
> +}
> +
> void usage(char *prog)
> {
> printf("Usage: %s [options] [path [offset]]\n", prog);
> printf(" -d Enable debug error output\n");
> printf("\n");
> - printf("This test requires a POWER8 or POWER9 CPU and either a ");
> - printf("usable framebuffer at /dev/fb0 or the path to usable ");
> - printf("cache-inhibited memory and optional offset to be provided\n");
> + printf("This test requires a POWER8, POWER9 or POWER10 CPU ");
> + printf("and either a usable framebuffer at /dev/fb0 or ");
> + printf("the path to usable cache inhibited memory and optional ");
> + printf("offset to be provided\n");
> }
>
> int main(int argc, char *argv[])
> @@ -573,10 +654,14 @@ int main(int argc, char *argv[])
> "test_alignment_handler_vsx_207");
> rc |= test_harness(test_alignment_handler_vsx_300,
> "test_alignment_handler_vsx_300");
> + rc |= test_harness(test_alignment_handler_vsx_prefix,
> + "test_alignment_handler_vsx_prefix");
> rc |= test_harness(test_alignment_handler_integer,
> "test_alignment_handler_integer");
> rc |= test_harness(test_alignment_handler_integer_206,
> "test_alignment_handler_integer_206");
> + rc |= test_harness(test_alignment_handler_integer_prefix,
> + "test_alignment_handler_integer_prefix");
> rc |= test_harness(test_alignment_handler_vmx,
> "test_alignment_handler_vmx");
> rc |= test_harness(test_alignment_handler_fp,
> @@ -585,5 +670,7 @@ int main(int argc, char *argv[])
> "test_alignment_handler_fp_205");
> rc |= test_harness(test_alignment_handler_fp_206,
> "test_alignment_handler_fp_206");
> + rc |= test_harness(test_alignment_handler_fp_prefix,
> + "test_alignment_handler_fp_prefix");
> return rc;
> }
> diff --git a/tools/testing/selftests/powerpc/include/instructions.h b/tools/testing/selftests/powerpc/include/instructions.h
> index f36061eb6f0f..4efa6314bd96 100644
> --- a/tools/testing/selftests/powerpc/include/instructions.h
> +++ b/tools/testing/selftests/powerpc/include/instructions.h
> @@ -66,4 +66,81 @@ static inline int paste_last(void *i)
> #define PPC_INST_PASTE __PASTE(0, 0, 0, 0)
> #define PPC_INST_PASTE_LAST __PASTE(0, 0, 1, 1)
>
> +/* This defines the prefixed load/store instructions */
> +#ifdef __ASSEMBLY__
> +# define stringify_in_c(...) __VA_ARGS__
> +#else
> +# define __stringify_in_c(...) #__VA_ARGS__
> +# define stringify_in_c(...) __stringify_in_c(__VA_ARGS__) " "
> +#endif
> +
> +#define __PPC_RA(a) (((a) & 0x1f) << 16)
> +#define __PPC_RS(s) (((s) & 0x1f) << 21)
> +#define __PPC_RT(t) __PPC_RS(t)
> +#define __PPC_PREFIX_R(r) (((r) & 0x1) << 20)
> +
> +#define PPC_PREFIX_MLS 0x06000000
> +#define PPC_PREFIX_8LS 0x04000000
> +
> +#define PPC_INST_LBZ 0x88000000
> +#define PPC_INST_LHZ 0xa0000000
> +#define PPC_INST_LHA 0xa8000000
> +#define PPC_INST_LWZ 0x80000000
> +#define PPC_INST_STB 0x98000000
> +#define PPC_INST_STH 0xb0000000
> +#define PPC_INST_STW 0x90000000
> +#define PPC_INST_STD 0xf8000000
> +#define PPC_INST_LFS 0xc0000000
> +#define PPC_INST_LFD 0xc8000000
> +#define PPC_INST_STFS 0xd0000000
> +#define PPC_INST_STFD 0xd8000000
> +
> +#define PREFIX_MLS(instr, t, a, r, d) stringify_in_c(.balign 64, , 4;) \
> + stringify_in_c(.long PPC_PREFIX_MLS | \
> + __PPC_PREFIX_R(r) | \
> + (((d) >> 16) & 0x3ffff);) \
> + stringify_in_c(.long (instr) | \
> + __PPC_RT(t) | \
> + __PPC_RA(a) | \
> + ((d) & 0xffff);\n)
> +
> +#define PREFIX_8LS(instr, t, a, r, d) stringify_in_c(.balign 64, , 4;) \
> + stringify_in_c(.long PPC_PREFIX_8LS | \
> + __PPC_PREFIX_R(r) | \
> + (((d) >> 16) & 0x3ffff);) \
> + stringify_in_c(.long (instr) | \
> + __PPC_RT(t) | \
> + __PPC_RA(a) | \
> + ((d) & 0xffff);\n)
> +
> +/* Prefixed Integer Load/Store instructions */
> +#define PLBZ(t, a, r, d) PREFIX_MLS(PPC_INST_LBZ, t, a, r, d)
> +#define PLHZ(t, a, r, d) PREFIX_MLS(PPC_INST_LHZ, t, a, r, d)
> +#define PLHA(t, a, r, d) PREFIX_MLS(PPC_INST_LHA, t, a, r, d)
> +#define PLWZ(t, a, r, d) PREFIX_MLS(PPC_INST_LWZ, t, a, r, d)
> +#define PLWA(t, a, r, d) PREFIX_8LS(0xa4000000, t, a, r, d)
> +#define PLD(t, a, r, d) PREFIX_8LS(0xe4000000, t, a, r, d)
> +#define PLQ(t, a, r, d) PREFIX_8LS(0xe0000000, t, a, r, d)
> +#define PSTB(s, a, r, d) PREFIX_MLS(PPC_INST_STB, s, a, r, d)
> +#define PSTH(s, a, r, d) PREFIX_MLS(PPC_INST_STH, s, a, r, d)
> +#define PSTW(s, a, r, d) PREFIX_MLS(PPC_INST_STW, s, a, r, d)
> +#define PSTD(s, a, r, d) PREFIX_8LS(0xf4000000, s, a, r, d)
> +#define PSTQ(s, a, r, d) PREFIX_8LS(0xf0000000, s, a, r, d)
> +
> +/* Prefixed Floating-Point Load/Store Instructions */
> +#define PLFS(frt, a, r, d) PREFIX_MLS(PPC_INST_LFS, frt, a, r, d)
> +#define PLFD(frt, a, r, d) PREFIX_MLS(PPC_INST_LFD, frt, a, r, d)
> +#define PSTFS(frs, a, r, d) PREFIX_MLS(PPC_INST_STFS, frs, a, r, d)
> +#define PSTFD(frs, a, r, d) PREFIX_MLS(PPC_INST_STFD, frs, a, r, d)
> +
> +/* Prefixed VSX Load/Store Instructions */
> +#define PLXSD(vrt, a, r, d) PREFIX_8LS(0xa8000000, vrt, a, r, d)
> +#define PLXSSP(vrt, a, r, d) PREFIX_8LS(0xac000000, vrt, a, r, d)
> +#define PLXV0(s, a, r, d) PREFIX_8LS(0xc8000000, s, a, r, d)
> +#define PLXV1(s, a, r, d) PREFIX_8LS(0xcc000000, s, a, r, d)
> +#define PSTXSD(vrs, a, r, d) PREFIX_8LS(0xb8000000, vrs, a, r, d)
> +#define PSTXSSP(vrs, a, r, d) PREFIX_8LS(0xbc000000, vrs, a, r, d)
> +#define PSTXV0(s, a, r, d) PREFIX_8LS(0xd8000000, s, a, r, d)
> +#define PSTXV1(s, a, r, d) PREFIX_8LS(0xdc000000, s, a, r, d)
> +
> #endif /* _SELFTESTS_POWERPC_INSTRUCTIONS_H */
> diff --git a/tools/testing/selftests/powerpc/include/utils.h b/tools/testing/selftests/powerpc/include/utils.h
> index e089a0c30d9a..eb91cf3561f8 100644
> --- a/tools/testing/selftests/powerpc/include/utils.h
> +++ b/tools/testing/selftests/powerpc/include/utils.h
> @@ -101,6 +101,11 @@ do { \
> #define PPC_FEATURE2_ARCH_3_00 0x00800000
> #endif
>
> +/* POWER10 feature */
> +#ifndef PPC_FEATURE2_ARCH_3_10
> +#define PPC_FEATURE2_ARCH_3_10 0x00040000
> +#endif
> +
> #if defined(__powerpc64__)
> #define UCONTEXT_NIA(UC) (UC)->uc_mcontext.gp_regs[PT_NIP]
> #define UCONTEXT_MSR(UC) (UC)->uc_mcontext.gp_regs[PT_MSR]
>
^ permalink raw reply
* Re: [PATCH v2 2/2] ASoC: fsl_spdif: Add support for imx6sx platform
From: Shengjiu Wang @ 2020-06-17 6:47 UTC (permalink / raw)
To: Nicolin Chen
Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Linux-ALSA, Timur Tabi, Xiubo Li, Fabio Estevam, Shengjiu Wang,
Liam Girdwood, Takashi Iwai, Rob Herring, Mark Brown,
linuxppc-dev, linux-kernel
In-Reply-To: <20200617062457.GA6411@Asurada-Nvidia>
On Wed, Jun 17, 2020 at 2:27 PM Nicolin Chen <nicoleotsuka@gmail.com> wrote:
>
> On Wed, Jun 17, 2020 at 12:30:17PM +0800, Shengjiu Wang wrote:
> > The one difference on imx6sx platform is that the root clock
> > is shared with ASRC module, so we add a new flags
> > "shared_root_clock" which means the root clock is independent,
>
> "shared" means "not independent", against "independent" ;)
>
> > then we will not do the clk_set_rate and clk_round_rate to avoid
> > impact ASRC module usage.
> >
> > As add a new flags, we include the soc specific data struct.
> >
> > Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
>
> Can add this once fixing the remaining comments:
>
> Reviewed-by: Nicolin Chen <nicoleotsuka@gmail.com>
>
> > +static inline bool fsl_spdif_can_set_clk_rate(struct fsl_spdif_priv *spdif,
> > + int clk)
>
> Can actually merge into single line as kernel has 100-character
> limit now, though 80-char is still preferable for a good coding
> style. But I think this one wouldn't be too bad at all.
>
> > @@ -421,7 +456,7 @@ static int spdif_set_sample_rate(struct snd_pcm_substream *substream,
> > sysclk_df = spdif_priv->sysclk_df[rate];
> >
> > /* Don't mess up the clocks from other modules */
>
> We can drop this comments now as it's out-of-date and the name of
> the new helper function is straightforward enough.
>
ok, will send v3.
best regards
wang shengjiu
^ permalink raw reply
* [PATCH v3 1/2] ASoC: bindings: fsl_spdif: Add new compatible string for imx6sx
From: Shengjiu Wang @ 2020-06-17 6:58 UTC (permalink / raw)
To: timur, nicoleotsuka, Xiubo.Lee, festevam, broonie, perex, tiwai,
alsa-devel, lgirdwood, robh+dt, devicetree
Cc: linuxppc-dev, linux-kernel
Add new compatible string "fsl,imx6sx-spdif" in the binding document.
And add compatible string "fsl,vf610-spdif" which was missed before.
Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
---
Documentation/devicetree/bindings/sound/fsl,spdif.txt | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/sound/fsl,spdif.txt b/Documentation/devicetree/bindings/sound/fsl,spdif.txt
index 8b324f82a782..e1365b0ee1e9 100644
--- a/Documentation/devicetree/bindings/sound/fsl,spdif.txt
+++ b/Documentation/devicetree/bindings/sound/fsl,spdif.txt
@@ -6,7 +6,11 @@ a fibre cable.
Required properties:
- - compatible : Compatible list, must contain "fsl,imx35-spdif".
+ - compatible : Compatible list, should contain one of the following
+ compatibles:
+ "fsl,imx35-spdif",
+ "fsl,vf610-spdif",
+ "fsl,imx6sx-spdif",
- reg : Offset and length of the register set for the device.
--
2.21.0
^ permalink raw reply related
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