* [PATCH v2] ASoC: atmel_ssc_dai: Remove wrong spinlock usage
From: Gregory CLEMENT @ 2019-09-18 10:03 UTC (permalink / raw)
To: Codrin Ciubotariu, Liam Girdwood, Mark Brown, alsa-devel,
linux-kernel
Cc: Alexandre Belloni, Ludovic Desroches, linux-arm-kernel,
Gregory CLEMENT
A potential bug was reported in the email "[BUG] atmel_ssc_dai: a
possible sleep-in-atomic bug in atmel_ssc_shutdown"[1]
Indeed in the function atmel_ssc_shutdown() free_irq() was called in a
critical section protected by spinlock.
However this spinlock is only used in atmel_ssc_shutdown() and
atmel_ssc_startup() functions. After further analysis, it occurred that
the call to these function are already protected by mutex used on the
calling functions.
Then we can remove the spinlock which will fix this bug as a side
effect. Thanks to this patch the following message disappears:
"BUG: sleeping function called from invalid context at
kernel/locking/mutex.c:909"
[1]: https://www.spinics.net/lists/alsa-devel/msg71286.html
Reviewed-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com>
---
Changelog:
v1 -> v2:
- Removed the spinlock from the atmel_ssc_info struct
- Added the Reviewed-by flag form Alex
sound/soc/atmel/atmel_ssc_dai.c | 12 ++----------
sound/soc/atmel/atmel_ssc_dai.h | 1 -
2 files changed, 2 insertions(+), 11 deletions(-)
diff --git a/sound/soc/atmel/atmel_ssc_dai.c b/sound/soc/atmel/atmel_ssc_dai.c
index 6f89483ac88c..786b48ae4905 100644
--- a/sound/soc/atmel/atmel_ssc_dai.c
+++ b/sound/soc/atmel/atmel_ssc_dai.c
@@ -116,19 +116,16 @@ static struct atmel_pcm_dma_params ssc_dma_params[NUM_SSC_DEVICES][2] = {
static struct atmel_ssc_info ssc_info[NUM_SSC_DEVICES] = {
{
.name = "ssc0",
- .lock = __SPIN_LOCK_UNLOCKED(ssc_info[0].lock),
.dir_mask = SSC_DIR_MASK_UNUSED,
.initialized = 0,
},
{
.name = "ssc1",
- .lock = __SPIN_LOCK_UNLOCKED(ssc_info[1].lock),
.dir_mask = SSC_DIR_MASK_UNUSED,
.initialized = 0,
},
{
.name = "ssc2",
- .lock = __SPIN_LOCK_UNLOCKED(ssc_info[2].lock),
.dir_mask = SSC_DIR_MASK_UNUSED,
.initialized = 0,
},
@@ -317,13 +314,10 @@ static int atmel_ssc_startup(struct snd_pcm_substream *substream,
snd_soc_dai_set_dma_data(dai, substream, dma_params);
- spin_lock_irq(&ssc_p->lock);
- if (ssc_p->dir_mask & dir_mask) {
- spin_unlock_irq(&ssc_p->lock);
+ if (ssc_p->dir_mask & dir_mask)
return -EBUSY;
- }
+
ssc_p->dir_mask |= dir_mask;
- spin_unlock_irq(&ssc_p->lock);
return 0;
}
@@ -355,7 +349,6 @@ static void atmel_ssc_shutdown(struct snd_pcm_substream *substream,
dir_mask = 1 << dir;
- spin_lock_irq(&ssc_p->lock);
ssc_p->dir_mask &= ~dir_mask;
if (!ssc_p->dir_mask) {
if (ssc_p->initialized) {
@@ -369,7 +362,6 @@ static void atmel_ssc_shutdown(struct snd_pcm_substream *substream,
ssc_p->cmr_div = ssc_p->tcmr_period = ssc_p->rcmr_period = 0;
ssc_p->forced_divider = 0;
}
- spin_unlock_irq(&ssc_p->lock);
/* Shutdown the SSC clock. */
pr_debug("atmel_ssc_dai: Stopping clock\n");
diff --git a/sound/soc/atmel/atmel_ssc_dai.h b/sound/soc/atmel/atmel_ssc_dai.h
index ae764cb541c7..3470b966e449 100644
--- a/sound/soc/atmel/atmel_ssc_dai.h
+++ b/sound/soc/atmel/atmel_ssc_dai.h
@@ -93,7 +93,6 @@ struct atmel_ssc_state {
struct atmel_ssc_info {
char *name;
struct ssc_device *ssc;
- spinlock_t lock; /* lock for dir_mask */
unsigned short dir_mask; /* 0=unused, 1=playback, 2=capture */
unsigned short initialized; /* true if SSC has been initialized */
unsigned short daifmt;
--
2.23.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [PATCH V6 2/2] mailbox: introduce ARM SMC based mailbox
From: Andre Przywara @ 2019-09-18 10:00 UTC (permalink / raw)
To: Peng Fan
Cc: mark.rutland@arm.com, devicetree@vger.kernel.org,
f.fainelli@gmail.com, jassisinghbrar@gmail.com,
linux-kernel@vger.kernel.org, robh+dt@kernel.org, dl-linux-imx,
sudeep.holla@arm.com, linux-arm-kernel@lists.infradead.org
In-Reply-To: <AM0PR04MB44813D62FF7E6762BB17460E888E0@AM0PR04MB4481.eurprd04.prod.outlook.com>
On Wed, 18 Sep 2019 09:09:25 +0000
Peng Fan <peng.fan@nxp.com> wrote:
Hi,
> > Subject: Re: [PATCH V6 2/2] mailbox: introduce ARM SMC based mailbox
> >
> > On Mon, 16 Sep 2019 09:44:41 +0000
> > Peng Fan <peng.fan@nxp.com> wrote:
> >
> > Hi,
> >
> > looks quite good now, some smaller comments below.
> > I think the only thing left is the "function ID passed by the client" topic.
> >
> > Have you tried using this interface using hvc, for instance in KVM or Xen? For
>
> No. I do not have that implementation in hypervisor.
OK, I was thinking about this, because this comes up from to time in Xen, for instance.
It would allow to pass exactly one "virtual" clock to a guest (using SCMI), which Xen could pass on to Dom0, which would translate this to an the actual clock.
I was just worried that having just *one* example implementation might be too little to have a proper generic interface.
I started to update my Allwinner ATF mailbox implementation (which was using SCPI), I will let you know when I managed to eventually test this.
> > instance to provide access to a clock for a passed-through platform device?
> > Another use case that pops up from time to time is GPIO for guests. This
> > sounds like a use case for using the register interface, also we could use the
> > hvc return value.
> >
> > > From: Peng Fan <peng.fan@nxp.com>
> > >
> > > This mailbox driver implements a mailbox which signals transmitted
> > > data via an ARM smc (secure monitor call) instruction. The mailbox
> > > receiver is implemented in firmware and can synchronously return data
> > > when it returns execution to the non-secure world again.
> > > An asynchronous receive path is not implemented.
> > > This allows the usage of a mailbox to trigger firmware actions on SoCs
> > > which either don't have a separate management processor or on which
> > > such a core is not available. A user of this mailbox could be the SCP
> > > interface.
> > >
> > > Modified from Andre Przywara's v2 patch
> > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore
> > > .kernel.org%2Fpatchwork%2Fpatch%2F812999%2F&data=02%7C01%7
> > Cpeng.fa
> > >
> > n%40nxp.com%7C58a1ed4078264d14958f08d73b95ed7e%7C686ea1d3bc2b
> > 4c6fa92cd
> > >
> > 99c5c301635%7C0%7C1%7C637043387484474825&sdata=Cp1zlhlpQbg
> > BsWu9ZDV
> > > RKkXmd6kvUR%2BtPO7EPR7YLpA%3D&reserved=0
> > >
> > > Cc: Andre Przywara <andre.przywara@arm.com>
> > > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > > ---
> > > drivers/mailbox/Kconfig | 7 ++
> > > drivers/mailbox/Makefile | 2 +
> > > drivers/mailbox/arm-smc-mailbox.c | 167
> > > ++++++++++++++++++++++++++++++++++++++
> > > 3 files changed, 176 insertions(+)
> > > create mode 100644 drivers/mailbox/arm-smc-mailbox.c
> > >
> > > diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig index
> > > ab4eb750bbdd..7707ee26251a 100644
> > > --- a/drivers/mailbox/Kconfig
> > > +++ b/drivers/mailbox/Kconfig
> > > @@ -16,6 +16,13 @@ config ARM_MHU
> > > The controller has 3 mailbox channels, the last of which can be
> > > used in Secure mode only.
> > >
> > > +config ARM_SMC_MBOX
> > > + tristate "Generic ARM smc mailbox"
> > > + depends on OF && HAVE_ARM_SMCCC
> > > + help
> > > + Generic mailbox driver which uses ARM smc calls to call into
> > > + firmware for triggering mailboxes.
> > > +
> > > config IMX_MBOX
> > > tristate "i.MX Mailbox"
> > > depends on ARCH_MXC || COMPILE_TEST
> > > diff --git a/drivers/mailbox/Makefile b/drivers/mailbox/Makefile index
> > > c22fad6f696b..93918a84c91b 100644
> > > --- a/drivers/mailbox/Makefile
> > > +++ b/drivers/mailbox/Makefile
> > > @@ -7,6 +7,8 @@ obj-$(CONFIG_MAILBOX_TEST) += mailbox-test.o
> > >
> > > obj-$(CONFIG_ARM_MHU) += arm_mhu.o
> > >
> > > +obj-$(CONFIG_ARM_SMC_MBOX) += arm-smc-mailbox.o
> > > +
> > > obj-$(CONFIG_IMX_MBOX) += imx-mailbox.o
> > >
> > > obj-$(CONFIG_ARMADA_37XX_RWTM_MBOX) +=
> > armada-37xx-rwtm-mailbox.o
> > > diff --git a/drivers/mailbox/arm-smc-mailbox.c
> > > b/drivers/mailbox/arm-smc-mailbox.c
> > > new file mode 100644
> > > index 000000000000..c84aef39c8d9
> > > --- /dev/null
> > > +++ b/drivers/mailbox/arm-smc-mailbox.c
> > > @@ -0,0 +1,167 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +/*
> > > + * Copyright (C) 2016,2017 ARM Ltd.
> > > + * Copyright 2019 NXP
> > > + */
> > > +
> > > +#include <linux/arm-smccc.h>
> > > +#include <linux/device.h>
> > > +#include <linux/kernel.h>
> > > +#include <linux/interrupt.h>
> > > +#include <linux/mailbox_controller.h> #include <linux/module.h>
> > > +#include <linux/platform_device.h>
> > > +
> > > +struct arm_smc_chan_data {
> > > + unsigned int function_id;
> > > +};
> > > +
> > > +struct arm_smccc_mbox_cmd {
> > > + unsigned int function_id;
> > > + union {
> > > + unsigned int args_smccc32[6];
> > > + unsigned long args_smccc64[6];
> >
> > Shouldn't this be u32 and u64 here, as the data type has this explicit length in
> > the SMCCC?
>
> ok
>
> >
> > > + };
> > > +};
> >
> > If this is the data structure that this mailbox controller uses, I would expect
> > this to be documented somewhere, or even exported.
>
> Export this structure in include/linux/mailbox/smc-mailbox.h?
For instance, even though I am not sure this is really desired or helpful, since we expect a generic mailbox client (the SCPI or SCMI driver) to deal with that mailbox.
But at least the expected format should be documented, which could just be in writing, not necessarily in code.
> > And again, I don't like the idea of having the function ID in here.
>
> You mean function_id in arm_smccc_mbox_cmd is not good?
Yes, this should not be something which mailbox clients are able to choose arbitrarily (see my other email).
> >
> > > +
> > > +typedef unsigned long (smc_mbox_fn)(unsigned int, unsigned long,
> > > + unsigned long, unsigned long,
> > > + unsigned long, unsigned long,
> > > + unsigned long);
> > > +static smc_mbox_fn *invoke_smc_mbox_fn;
> > > +
> > > +static int arm_smc_send_data(struct mbox_chan *link, void *data) {
> > > + struct arm_smc_chan_data *chan_data = link->con_priv;
> > > + struct arm_smccc_mbox_cmd *cmd = data;
> > > + unsigned long ret;
> > > + u32 function_id;
> > > +
> > > + function_id = chan_data->function_id;
> > > + if (!function_id)
> > > + function_id = cmd->function_id;
> > > +
> > > + if (function_id & BIT(30)) {
> >
> > if (ARM_SMCCC_IS_64(function_id)) {
>
> ok
>
> >
> > > + ret = invoke_smc_mbox_fn(function_id, cmd->args_smccc64[0],
> > > + cmd->args_smccc64[1],
> > > + cmd->args_smccc64[2],
> > > + cmd->args_smccc64[3],
> > > + cmd->args_smccc64[4],
> > > + cmd->args_smccc64[5]);
> > > + } else {
> > > + ret = invoke_smc_mbox_fn(function_id, cmd->args_smccc32[0],
> > > + cmd->args_smccc32[1],
> > > + cmd->args_smccc32[2],
> > > + cmd->args_smccc32[3],
> > > + cmd->args_smccc32[4],
> > > + cmd->args_smccc32[5]);
> > > + }
> > > +
> > > + mbox_chan_received_data(link, (void *)ret);
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static unsigned long __invoke_fn_hvc(unsigned int function_id,
> > > + unsigned long arg0, unsigned long arg1,
> > > + unsigned long arg2, unsigned long arg3,
> > > + unsigned long arg4, unsigned long arg5) {
> > > + struct arm_smccc_res res;
> > > +
> > > + arm_smccc_hvc(function_id, arg0, arg1, arg2, arg3, arg4,
> > > + arg5, 0, &res);
> > > + return res.a0;
> > > +}
> > > +
> > > +static unsigned long __invoke_fn_smc(unsigned int function_id,
> > > + unsigned long arg0, unsigned long arg1,
> > > + unsigned long arg2, unsigned long arg3,
> > > + unsigned long arg4, unsigned long arg5) {
> > > + struct arm_smccc_res res;
> > > +
> > > + arm_smccc_smc(function_id, arg0, arg1, arg2, arg3, arg4,
> > > + arg5, 0, &res);
> > > + return res.a0;
> > > +}
> > > +
> > > +static const struct mbox_chan_ops arm_smc_mbox_chan_ops = {
> > > + .send_data = arm_smc_send_data,
> > > +};
> > > +
> > > +static int arm_smc_mbox_probe(struct platform_device *pdev) {
> > > + struct device *dev = &pdev->dev;
> > > + struct mbox_controller *mbox;
> > > + struct arm_smc_chan_data *chan_data;
> > > + int ret;
> > > + u32 function_id = 0;
> > > +
> > > + if (of_device_is_compatible(dev->of_node, "arm,smc-mbox"))
> > > + invoke_smc_mbox_fn = __invoke_fn_smc;
> > > + else
> > > + invoke_smc_mbox_fn = __invoke_fn_hvc;
> > > +
> > > + mbox = devm_kzalloc(dev, sizeof(*mbox), GFP_KERNEL);
> > > + if (!mbox)
> > > + return -ENOMEM;
> > > +
> > > + mbox->num_chans = 1;
> > > + mbox->chans = devm_kzalloc(dev, sizeof(*mbox->chans), GFP_KERNEL);
> > > + if (!mbox->chans)
> > > + return -ENOMEM;
> > > +
> > > + chan_data = devm_kzalloc(dev, sizeof(*chan_data), GFP_KERNEL);
> > > + if (!chan_data)
> > > + return -ENOMEM;
> > > +
> > > + of_property_read_u32(dev->of_node, "arm,func-id", &function_id);
> > > + chan_data->function_id = function_id;
> > > +
> > > + mbox->chans->con_priv = chan_data;
> > > +
> > > + mbox->txdone_poll = false;
> > > + mbox->txdone_irq = false;
> >
> > Don't we need to provide something to confirm reception to the client? In our
> > case we can set this as soon as the smc/hvc returns.
>
> As smc/hvc returns, it means the transfer is over and receive is done.
I understand that, but was wondering if the Linux mailbox framework knows about that? In my older version I was calling mbox_chan_received_data() after the smc call returned.
Also there is mbox_chan_txdone() with which a controller driver can signal TX completion explicitly.
Cheers,
Andre.
> > Cheers,
> > Andre.
> >
> > > + mbox->ops = &arm_smc_mbox_chan_ops;
> > > + mbox->dev = dev;
> > > +
> > > + platform_set_drvdata(pdev, mbox);
> > > +
> > > + ret = devm_mbox_controller_register(dev, mbox);
> > > + if (ret)
> > > + return ret;
> > > +
> > > + dev_info(dev, "ARM SMC mailbox enabled.\n");
> > > +
> > > + return ret;
> > > +}
> > > +
> > > +static int arm_smc_mbox_remove(struct platform_device *pdev) {
> > > + struct mbox_controller *mbox = platform_get_drvdata(pdev);
> > > +
> > > + mbox_controller_unregister(mbox);
> > > + return 0;
> > > +}
> > > +
> > > +static const struct of_device_id arm_smc_mbox_of_match[] = {
> > > + { .compatible = "arm,smc-mbox", },
> > > + { .compatible = "arm,hvc-mbox", },
> > > + {},
> > > +};
> > > +MODULE_DEVICE_TABLE(of, arm_smc_mbox_of_match);
> > > +
> > > +static struct platform_driver arm_smc_mbox_driver = {
> > > + .driver = {
> > > + .name = "arm-smc-mbox",
> > > + .of_match_table = arm_smc_mbox_of_match,
> > > + },
> > > + .probe = arm_smc_mbox_probe,
> > > + .remove = arm_smc_mbox_remove,
> > > +};
> > > +module_platform_driver(arm_smc_mbox_driver);
> > > +
> > > +MODULE_AUTHOR("Peng Fan <peng.fan@nxp.com>");
> > > +MODULE_DESCRIPTION("Generic ARM smc mailbox driver");
> > > +MODULE_LICENSE("GPL v2");
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH] ASoC: atmel_ssc_dai: Remove wrong spinlock usage
From: Gregory CLEMENT @ 2019-09-18 9:59 UTC (permalink / raw)
To: Alexandre Belloni
Cc: alsa-devel, linux-kernel, Liam Girdwood, Ludovic Desroches,
Mark Brown, Codrin Ciubotariu, linux-arm-kernel
In-Reply-To: <20190918094800.GJ21254@piout.net>
Hi Alex,
> On 18/09/2019 11:41:14+0200, Gregory CLEMENT wrote:
>> A potential bug was reported in the email "[BUG] atmel_ssc_dai: a
>> possible sleep-in-atomic bug in atmel_ssc_shutdown"[1]
>>
>> Indeed in the function atmel_ssc_shutdown() free_irq() was called in a
>> critical section protected by spinlock.
>>
>> However this spinlock is only used in atmel_ssc_shutdown() and
>> atmel_ssc_startup() functions. After further analysis, it occurred that
>> the call to these function are already protected by mutex used on the
>> calling functions.
>>
>> Then we can remove the spinlock which will fix this bug as a side
>> effect. Thanks to this patch the following message disappears:
>>
>> "BUG: sleeping function called from invalid context at
>> kernel/locking/mutex.c:909"
>>
>> [1]: https://www.spinics.net/lists/alsa-devel/msg71286.html
>>
>> Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com>
>> ---
>> sound/soc/atmel/atmel_ssc_dai.c | 7 -------
>> 1 file changed, 7 deletions(-)
>>
>> diff --git a/sound/soc/atmel/atmel_ssc_dai.c b/sound/soc/atmel/atmel_ssc_dai.c
>> index 6f89483ac88c..365957e86419 100644
>> --- a/sound/soc/atmel/atmel_ssc_dai.c
>> +++ b/sound/soc/atmel/atmel_ssc_dai.c
>> @@ -116,19 +116,16 @@ static struct atmel_pcm_dma_params ssc_dma_params[NUM_SSC_DEVICES][2] = {
>> static struct atmel_ssc_info ssc_info[NUM_SSC_DEVICES] = {
>> {
>> .name = "ssc0",
>> - .lock = __SPIN_LOCK_UNLOCKED(ssc_info[0].lock),
>
> This member is now unused and can be removed from the struct.
>
> Once fixed, Reviewed-by: Alexandre Belloni
> <alexandre.belloni@bootlin.com>
Indeed I wanted to do this but finally forgot.
I am sending a v2.
Thanks,
Gregory
>
>> .dir_mask = SSC_DIR_MASK_UNUSED,
>> .initialized = 0,
>> },
>> {
>> .name = "ssc1",
>> - .lock = __SPIN_LOCK_UNLOCKED(ssc_info[1].lock),
>> .dir_mask = SSC_DIR_MASK_UNUSED,
>> .initialized = 0,
>> },
>> {
>> .name = "ssc2",
>> - .lock = __SPIN_LOCK_UNLOCKED(ssc_info[2].lock),
>> .dir_mask = SSC_DIR_MASK_UNUSED,
>> .initialized = 0,
>> },
>> @@ -317,13 +314,11 @@ static int atmel_ssc_startup(struct snd_pcm_substream *substream,
>>
>> snd_soc_dai_set_dma_data(dai, substream, dma_params);
>>
>> - spin_lock_irq(&ssc_p->lock);
>> if (ssc_p->dir_mask & dir_mask) {
>> spin_unlock_irq(&ssc_p->lock);
>> return -EBUSY;
>> }
>> ssc_p->dir_mask |= dir_mask;
>> - spin_unlock_irq(&ssc_p->lock);
>>
>> return 0;
>> }
>> @@ -355,7 +350,6 @@ static void atmel_ssc_shutdown(struct snd_pcm_substream *substream,
>>
>> dir_mask = 1 << dir;
>>
>> - spin_lock_irq(&ssc_p->lock);
>> ssc_p->dir_mask &= ~dir_mask;
>> if (!ssc_p->dir_mask) {
>> if (ssc_p->initialized) {
>> @@ -369,7 +363,6 @@ static void atmel_ssc_shutdown(struct snd_pcm_substream *substream,
>> ssc_p->cmr_div = ssc_p->tcmr_period = ssc_p->rcmr_period = 0;
>> ssc_p->forced_divider = 0;
>> }
>> - spin_unlock_irq(&ssc_p->lock);
>>
>> /* Shutdown the SSC clock. */
>> pr_debug("atmel_ssc_dai: Stopping clock\n");
>> --
>> 2.23.0
>>
>
> --
> Alexandre Belloni, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
--
Gregory Clement, Bootlin
Embedded Linux and Kernel engineering
http://bootlin.com
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [Question-GIC-v4.1] Plan on GIC-v4.1 driver development
From: Marc Zyngier @ 2019-09-18 9:58 UTC (permalink / raw)
To: Shaokun Zhang, linux-arm-kernel, kvmarm; +Cc: Tangnianyao (ICT)
In-Reply-To: <0501c049-67a6-9a63-e0d9-e043573e1211@kernel.org>
On 17/09/2019 10:23, Marc Zyngier wrote:
> On 17/09/2019 03:15, Shaokun Zhang wrote:
>> Hi Marc,
>>
>> This is from Nianyao Tang.
>>
>> I'm planning to do some verification on our GIC-v4.1 implement. I would like some
>> information about linux GIC-v4.1 driver. When will linux support GIC-v4.1 or what's
>> the plan on developing GIC-v4.1 driver?
>
> The easy answer is that yes, there is a plan. There is some code, even,
> just not quite in a usable state yet. I'll try to push something out
> once I get a chance.
FWIW, I've pushed out a branch containing the current state of this work
at [1]. It doesn't really work at the moment (Doorbells and SGIs are
unreliable), but hopefully that will give you an idea of what is going on.
Being a work in progress, it is unstable, subject to being changed,
rebased and deleted without warning.
Thanks,
M.
[1]
https://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git/log/?h=irq/gic-v4.1-devel
--
Jazz is not dead, it just smells funny...
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* RE: [RFC PATCH v3 4/6] psci: Add hvc call service for ptp_kvm.
From: Jianyong Wu (Arm Technology China) @ 2019-09-18 9:57 UTC (permalink / raw)
To: Paolo Bonzini, netdev@vger.kernel.org, yangbo.lu@nxp.com,
john.stultz@linaro.org, tglx@linutronix.de,
sean.j.christopherson@intel.com, maz@kernel.org,
richardcochran@gmail.com, Mark Rutland, Will Deacon,
Suzuki Poulose
Cc: Justin He (Arm Technology China), kvm@vger.kernel.org,
Steve Capper, linux-kernel@vger.kernel.org,
Kaly Xin (Arm Technology China), nd,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <83ed7fac-277f-a31e-af37-8ec134f39d26@redhat.com>
Hi Paolo,
> -----Original Message-----
> From: Paolo Bonzini <pbonzini@redhat.com>
> Sent: Wednesday, September 18, 2019 4:26 PM
> To: Jianyong Wu (Arm Technology China) <Jianyong.Wu@arm.com>;
> netdev@vger.kernel.org; yangbo.lu@nxp.com; john.stultz@linaro.org;
> tglx@linutronix.de; sean.j.christopherson@intel.com; maz@kernel.org;
> richardcochran@gmail.com; Mark Rutland <Mark.Rutland@arm.com>; Will
> Deacon <Will.Deacon@arm.com>; Suzuki Poulose
> <Suzuki.Poulose@arm.com>
> Cc: linux-kernel@vger.kernel.org; kvm@vger.kernel.org; Steve Capper
> <Steve.Capper@arm.com>; Kaly Xin (Arm Technology China)
> <Kaly.Xin@arm.com>; Justin He (Arm Technology China)
> <Justin.He@arm.com>; nd <nd@arm.com>; linux-arm-
> kernel@lists.infradead.org
> Subject: Re: [RFC PATCH v3 4/6] psci: Add hvc call service for ptp_kvm.
>
> On 18/09/19 10:07, Jianyong Wu wrote:
> > + case ARM_SMCCC_VENDOR_HYP_KVM_PTP_FUNC_ID:
> > + getnstimeofday(ts);
>
> This is not Y2038-safe. Please use ktime_get_real_ts64 instead, and split the
> 64-bit seconds value between val[0] and val[1].
>
As far as I know, y2038-safe will only affect signed 32-bit integer, how does it affect 64-bit integer?
And why split 64-bit number into two blocks is necessary?
Also, split number will lead to shortage of return value.
> However, it seems to me that the new function is not needed and you can
> just use ktime_get_snapshot. You'll get the time in systime_snapshot->real
> and the cycles value in systime_snapshot->cycles.
See patch 5/6, I need both counter cycle and clocksource, ktime_get_snapshot seems only offer cycles.
>
> > + get_current_counterval(&sc);
> > + val[0] = ts->tv_sec;
> > + val[1] = ts->tv_nsec;
> > + val[2] = sc.cycles;
> > + val[3] = 0;
> > + break;
>
> This should return a guest-cycles value. If the cycles values always the same
> between the host and the guest on ARM, then okay. If not, you have to
> apply whatever offset exists.
>
In my opinion, when use ptp_kvm as clock sources to sync time between host and guest, user should promise the guest and host has no
clock offset. So we can be sure that the cycle between guest and host should be keep consistent. But I need check it.
I think host cycle should be returned to guest as we should promise we get clock and counter in the same time.
Thanks
Jianyong Wu
> Thanks,
>
> Paolo
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH] mfd: mt6360: add pmic mt6360 driver
From: Gene Chen @ 2019-09-18 9:55 UTC (permalink / raw)
To: lee.jones, matthias.bgg
Cc: gene_chen, linux-kernel, linux-mediatek, Gene Chen, Wilma.Wu,
linux-arm-kernel
From: Gene Chen <gene_chen@mediatek.corp-partner.google.com>
Add mfd driver for mt6360 pmic chip include
Battery Charger/USB_PD/Flash LED/RGB LED/LDO/Buck
Signed-off-by: Gene Chen <gene_chen@richtek.com>
---
drivers/mfd/Kconfig | 12 ++
drivers/mfd/Makefile | 1 +
drivers/mfd/mt6360-core.c | 463 ++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 476 insertions(+)
create mode 100644 drivers/mfd/mt6360-core.c
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index f129f96..a422c76 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -862,6 +862,18 @@ config MFD_MAX8998
additional drivers must be enabled in order to use the functionality
of the device.
+config MFD_MT6360
+ tristate "Mediatek MT6360 SubPMIC"
+ select MFD_CORE
+ select REGMAP_I2C
+ select REGMAP_IRQ
+ depends on I2C
+ help
+ Say Y here to enable MT6360 PMU/PMIC/LDO functional support.
+ PMU part include charger, flashlight, rgb led
+ PMIC part include 2-channel BUCKs and 2-channel LDOs
+ LDO part include 4-channel LDOs
+
config MFD_MT6397
tristate "MediaTek MT6397 PMIC Support"
select MFD_CORE
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index f026ada..77a8f0b 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -241,6 +241,7 @@ obj-$(CONFIG_INTEL_SOC_PMIC) += intel-soc-pmic.o
obj-$(CONFIG_INTEL_SOC_PMIC_BXTWC) += intel_soc_pmic_bxtwc.o
obj-$(CONFIG_INTEL_SOC_PMIC_CHTWC) += intel_soc_pmic_chtwc.o
obj-$(CONFIG_INTEL_SOC_PMIC_CHTDC_TI) += intel_soc_pmic_chtdc_ti.o
+obj-$(CONFIG_MFD_MT6360) += mt6360-core.o
obj-$(CONFIG_MFD_MT6397) += mt6397-core.o
obj-$(CONFIG_MFD_ALTERA_A10SR) += altera-a10sr.o
diff --git a/drivers/mfd/mt6360-core.c b/drivers/mfd/mt6360-core.c
new file mode 100644
index 0000000..d3580618
--- /dev/null
+++ b/drivers/mfd/mt6360-core.c
@@ -0,0 +1,463 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2019 MediaTek Inc.
+ */
+
+#include <linux/i2c.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/mfd/core.h>
+#include <linux/module.h>
+#include <linux/of_irq.h>
+#include <linux/of_platform.h>
+#include <linux/version.h>
+
+#include <linux/mfd/mt6360.h>
+#include <linux/mfd/mt6360-private.h>
+
+/* reg 0 -> 0 ~ 7 */
+#define MT6360_CHG_TREG_EVT (4)
+#define MT6360_CHG_AICR_EVT (5)
+#define MT6360_CHG_MIVR_EVT (6)
+#define MT6360_PWR_RDY_EVT (7)
+/* REG 1 -> 8 ~ 15 */
+#define MT6360_CHG_BATSYSUV_EVT (9)
+#define MT6360_FLED_CHG_VINOVP_EVT (11)
+#define MT6360_CHG_VSYSUV_EVT (12)
+#define MT6360_CHG_VSYSOV_EVT (13)
+#define MT6360_CHG_VBATOV_EVT (14)
+#define MT6360_CHG_VBUSOV_EVT (15)
+/* REG 2 -> 16 ~ 23 */
+/* REG 3 -> 24 ~ 31 */
+#define MT6360_WD_PMU_DET (25)
+#define MT6360_WD_PMU_DONE (26)
+#define MT6360_CHG_TMRI (27)
+#define MT6360_CHG_ADPBADI (29)
+#define MT6360_CHG_RVPI (30)
+#define MT6360_OTPI (31)
+/* REG 4 -> 32 ~ 39 */
+#define MT6360_CHG_AICCMEASL (32)
+#define MT6360_CHGDET_DONEI (34)
+#define MT6360_WDTMRI (35)
+#define MT6360_SSFINISHI (36)
+#define MT6360_CHG_RECHGI (37)
+#define MT6360_CHG_TERMI (38)
+#define MT6360_CHG_IEOCI (39)
+/* REG 5 -> 40 ~ 47 */
+#define MT6360_PUMPX_DONEI (40)
+#define MT6360_BAT_OVP_ADC_EVT (41)
+#define MT6360_TYPEC_OTP_EVT (42)
+#define MT6360_ADC_WAKEUP_EVT (43)
+#define MT6360_ADC_DONEI (44)
+#define MT6360_BST_BATUVI (45)
+#define MT6360_BST_VBUSOVI (46)
+#define MT6360_BST_OLPI (47)
+/* REG 6 -> 48 ~ 55 */
+#define MT6360_ATTACH_I (48)
+#define MT6360_DETACH_I (49)
+#define MT6360_QC30_STPDONE (51)
+#define MT6360_QC_VBUSDET_DONE (52)
+#define MT6360_HVDCP_DET (53)
+#define MT6360_CHGDETI (54)
+#define MT6360_DCDTI (55)
+/* REG 7 -> 56 ~ 63 */
+#define MT6360_FOD_DONE_EVT (56)
+#define MT6360_FOD_OV_EVT (57)
+#define MT6360_CHRDET_UVP_EVT (58)
+#define MT6360_CHRDET_OVP_EVT (59)
+#define MT6360_CHRDET_EXT_EVT (60)
+#define MT6360_FOD_LR_EVT (61)
+#define MT6360_FOD_HR_EVT (62)
+#define MT6360_FOD_DISCHG_FAIL_EVT (63)
+/* REG 8 -> 64 ~ 71 */
+#define MT6360_USBID_EVT (64)
+#define MT6360_APWDTRST_EVT (65)
+#define MT6360_EN_EVT (66)
+#define MT6360_QONB_RST_EVT (67)
+#define MT6360_MRSTB_EVT (68)
+#define MT6360_OTP_EVT (69)
+#define MT6360_VDDAOV_EVT (70)
+#define MT6360_SYSUV_EVT (71)
+/* REG 9 -> 72 ~ 79 */
+#define MT6360_FLED_STRBPIN_EVT (72)
+#define MT6360_FLED_TORPIN_EVT (73)
+#define MT6360_FLED_TX_EVT (74)
+#define MT6360_FLED_LVF_EVT (75)
+#define MT6360_FLED2_SHORT_EVT (78)
+#define MT6360_FLED1_SHORT_EVT (79)
+/* REG 10 -> 80 ~ 87 */
+#define MT6360_FLED2_STRB_EVT (80)
+#define MT6360_FLED1_STRB_EVT (81)
+#define MT6360_FLED2_STRB_TO_EVT (82)
+#define MT6360_FLED1_STRB_TO_EVT (83)
+#define MT6360_FLED2_TOR_EVT (84)
+#define MT6360_FLED1_TOR_EVT (85)
+/* REG 11 -> 88 ~ 95 */
+/* REG 12 -> 96 ~ 103 */
+#define MT6360_BUCK1_PGB_EVT (96)
+#define MT6360_BUCK1_OC_EVT (100)
+#define MT6360_BUCK1_OV_EVT (101)
+#define MT6360_BUCK1_UV_EVT (102)
+/* REG 13 -> 104 ~ 111 */
+#define MT6360_BUCK2_PGB_EVT (104)
+#define MT6360_BUCK2_OC_EVT (108)
+#define MT6360_BUCK2_OV_EVT (109)
+#define MT6360_BUCK2_UV_EVT (110)
+/* REG 14 -> 112 ~ 119 */
+#define MT6360_LDO1_OC_EVT (113)
+#define MT6360_LDO2_OC_EVT (114)
+#define MT6360_LDO3_OC_EVT (115)
+#define MT6360_LDO5_OC_EVT (117)
+#define MT6360_LDO6_OC_EVT (118)
+#define MT6360_LDO7_OC_EVT (119)
+/* REG 15 -> 120 ~ 127 */
+#define MT6360_LDO1_PGB_EVT (121)
+#define MT6360_LDO2_PGB_EVT (122)
+#define MT6360_LDO3_PGB_EVT (123)
+#define MT6360_LDO5_PGB_EVT (125)
+#define MT6360_LDO6_PGB_EVT (126)
+#define MT6360_LDO7_PGB_EVT (127)
+
+#define MT6360_REGMAP_IRQ_REG(_irq_evt) \
+ REGMAP_IRQ_REG(_irq_evt, (_irq_evt) / 8, BIT((_irq_evt) % 8))
+
+#define MT6360_MFD_CELL(_name) \
+ { \
+ .name = #_name, \
+ .of_compatible = "mediatek," #_name, \
+ .num_resources = ARRAY_SIZE(_name##_resources), \
+ .resources = _name##_resources, \
+ }
+
+static const struct regmap_irq mt6360_pmu_irqs[] = {
+ MT6360_REGMAP_IRQ_REG(MT6360_CHG_TREG_EVT),
+ MT6360_REGMAP_IRQ_REG(MT6360_CHG_AICR_EVT),
+ MT6360_REGMAP_IRQ_REG(MT6360_CHG_MIVR_EVT),
+ MT6360_REGMAP_IRQ_REG(MT6360_PWR_RDY_EVT),
+ MT6360_REGMAP_IRQ_REG(MT6360_CHG_BATSYSUV_EVT),
+ MT6360_REGMAP_IRQ_REG(MT6360_FLED_CHG_VINOVP_EVT),
+ MT6360_REGMAP_IRQ_REG(MT6360_CHG_VSYSUV_EVT),
+ MT6360_REGMAP_IRQ_REG(MT6360_CHG_VSYSOV_EVT),
+ MT6360_REGMAP_IRQ_REG(MT6360_CHG_VBATOV_EVT),
+ MT6360_REGMAP_IRQ_REG(MT6360_CHG_VBUSOV_EVT),
+ MT6360_REGMAP_IRQ_REG(MT6360_WD_PMU_DET),
+ MT6360_REGMAP_IRQ_REG(MT6360_WD_PMU_DONE),
+ MT6360_REGMAP_IRQ_REG(MT6360_CHG_TMRI),
+ MT6360_REGMAP_IRQ_REG(MT6360_CHG_ADPBADI),
+ MT6360_REGMAP_IRQ_REG(MT6360_CHG_RVPI),
+ MT6360_REGMAP_IRQ_REG(MT6360_OTPI),
+ MT6360_REGMAP_IRQ_REG(MT6360_CHG_AICCMEASL),
+ MT6360_REGMAP_IRQ_REG(MT6360_CHGDET_DONEI),
+ MT6360_REGMAP_IRQ_REG(MT6360_WDTMRI),
+ MT6360_REGMAP_IRQ_REG(MT6360_SSFINISHI),
+ MT6360_REGMAP_IRQ_REG(MT6360_CHG_RECHGI),
+ MT6360_REGMAP_IRQ_REG(MT6360_CHG_TERMI),
+ MT6360_REGMAP_IRQ_REG(MT6360_CHG_IEOCI),
+ MT6360_REGMAP_IRQ_REG(MT6360_PUMPX_DONEI),
+ MT6360_REGMAP_IRQ_REG(MT6360_CHG_TREG_EVT),
+ MT6360_REGMAP_IRQ_REG(MT6360_BAT_OVP_ADC_EVT),
+ MT6360_REGMAP_IRQ_REG(MT6360_TYPEC_OTP_EVT),
+ MT6360_REGMAP_IRQ_REG(MT6360_ADC_WAKEUP_EVT),
+ MT6360_REGMAP_IRQ_REG(MT6360_ADC_DONEI),
+ MT6360_REGMAP_IRQ_REG(MT6360_BST_BATUVI),
+ MT6360_REGMAP_IRQ_REG(MT6360_BST_VBUSOVI),
+ MT6360_REGMAP_IRQ_REG(MT6360_BST_OLPI),
+ MT6360_REGMAP_IRQ_REG(MT6360_ATTACH_I),
+ MT6360_REGMAP_IRQ_REG(MT6360_DETACH_I),
+ MT6360_REGMAP_IRQ_REG(MT6360_QC30_STPDONE),
+ MT6360_REGMAP_IRQ_REG(MT6360_QC_VBUSDET_DONE),
+ MT6360_REGMAP_IRQ_REG(MT6360_HVDCP_DET),
+ MT6360_REGMAP_IRQ_REG(MT6360_CHGDETI),
+ MT6360_REGMAP_IRQ_REG(MT6360_DCDTI),
+ MT6360_REGMAP_IRQ_REG(MT6360_FOD_DONE_EVT),
+ MT6360_REGMAP_IRQ_REG(MT6360_FOD_OV_EVT),
+ MT6360_REGMAP_IRQ_REG(MT6360_CHRDET_UVP_EVT),
+ MT6360_REGMAP_IRQ_REG(MT6360_CHRDET_OVP_EVT),
+ MT6360_REGMAP_IRQ_REG(MT6360_CHRDET_EXT_EVT),
+ MT6360_REGMAP_IRQ_REG(MT6360_FOD_LR_EVT),
+ MT6360_REGMAP_IRQ_REG(MT6360_FOD_HR_EVT),
+ MT6360_REGMAP_IRQ_REG(MT6360_FOD_DISCHG_FAIL_EVT),
+ MT6360_REGMAP_IRQ_REG(MT6360_USBID_EVT),
+ MT6360_REGMAP_IRQ_REG(MT6360_APWDTRST_EVT),
+ MT6360_REGMAP_IRQ_REG(MT6360_EN_EVT),
+ MT6360_REGMAP_IRQ_REG(MT6360_QONB_RST_EVT),
+ MT6360_REGMAP_IRQ_REG(MT6360_MRSTB_EVT),
+ MT6360_REGMAP_IRQ_REG(MT6360_OTP_EVT),
+ MT6360_REGMAP_IRQ_REG(MT6360_VDDAOV_EVT),
+ MT6360_REGMAP_IRQ_REG(MT6360_SYSUV_EVT),
+ MT6360_REGMAP_IRQ_REG(MT6360_FLED_STRBPIN_EVT),
+ MT6360_REGMAP_IRQ_REG(MT6360_FLED_TORPIN_EVT),
+ MT6360_REGMAP_IRQ_REG(MT6360_FLED_TX_EVT),
+ MT6360_REGMAP_IRQ_REG(MT6360_FLED_LVF_EVT),
+ MT6360_REGMAP_IRQ_REG(MT6360_FLED2_SHORT_EVT),
+ MT6360_REGMAP_IRQ_REG(MT6360_FLED1_SHORT_EVT),
+ MT6360_REGMAP_IRQ_REG(MT6360_FLED2_STRB_EVT),
+ MT6360_REGMAP_IRQ_REG(MT6360_FLED1_STRB_EVT),
+ MT6360_REGMAP_IRQ_REG(MT6360_FLED2_STRB_TO_EVT),
+ MT6360_REGMAP_IRQ_REG(MT6360_FLED1_STRB_TO_EVT),
+ MT6360_REGMAP_IRQ_REG(MT6360_FLED2_TOR_EVT),
+ MT6360_REGMAP_IRQ_REG(MT6360_FLED1_TOR_EVT),
+ MT6360_REGMAP_IRQ_REG(MT6360_BUCK1_PGB_EVT),
+ MT6360_REGMAP_IRQ_REG(MT6360_BUCK1_OC_EVT),
+ MT6360_REGMAP_IRQ_REG(MT6360_BUCK1_OV_EVT),
+ MT6360_REGMAP_IRQ_REG(MT6360_BUCK1_UV_EVT),
+ MT6360_REGMAP_IRQ_REG(MT6360_BUCK2_PGB_EVT),
+ MT6360_REGMAP_IRQ_REG(MT6360_BUCK2_OC_EVT),
+ MT6360_REGMAP_IRQ_REG(MT6360_BUCK2_OV_EVT),
+ MT6360_REGMAP_IRQ_REG(MT6360_BUCK2_UV_EVT),
+ MT6360_REGMAP_IRQ_REG(MT6360_LDO1_OC_EVT),
+ MT6360_REGMAP_IRQ_REG(MT6360_LDO2_OC_EVT),
+ MT6360_REGMAP_IRQ_REG(MT6360_LDO3_OC_EVT),
+ MT6360_REGMAP_IRQ_REG(MT6360_LDO5_OC_EVT),
+ MT6360_REGMAP_IRQ_REG(MT6360_LDO6_OC_EVT),
+ MT6360_REGMAP_IRQ_REG(MT6360_LDO7_OC_EVT),
+ MT6360_REGMAP_IRQ_REG(MT6360_LDO1_PGB_EVT),
+ MT6360_REGMAP_IRQ_REG(MT6360_LDO2_PGB_EVT),
+ MT6360_REGMAP_IRQ_REG(MT6360_LDO3_PGB_EVT),
+ MT6360_REGMAP_IRQ_REG(MT6360_LDO5_PGB_EVT),
+ MT6360_REGMAP_IRQ_REG(MT6360_LDO6_PGB_EVT),
+ MT6360_REGMAP_IRQ_REG(MT6360_LDO7_PGB_EVT),
+};
+
+static int mt6360_pmu_handle_post_irq(void *irq_drv_data)
+{
+ struct mt6360_pmu_info *mpi = irq_drv_data;
+
+ return regmap_update_bits(mpi->regmap,
+ MT6360_PMU_IRQ_SET, MT6360_IRQ_RETRIG, MT6360_IRQ_RETRIG);
+}
+
+static const struct regmap_irq_chip mt6360_pmu_irq_chip = {
+ .irqs = mt6360_pmu_irqs,
+ .num_irqs = ARRAY_SIZE(mt6360_pmu_irqs),
+ .num_regs = MT6360_PMU_IRQ_REGNUM,
+ .mask_base = MT6360_PMU_CHG_MASK1,
+ .status_base = MT6360_PMU_CHG_IRQ1,
+ .ack_base = MT6360_PMU_CHG_IRQ1,
+ .init_ack_masked = true,
+ .use_ack = true,
+ .handle_post_irq = mt6360_pmu_handle_post_irq,
+};
+
+static const struct regmap_config mt6360_pmu_regmap_config = {
+ .reg_bits = 8,
+ .val_bits = 8,
+ .max_register = MT6360_PMU_MAXREG,
+};
+
+static const struct resource mt6360_adc_resources[] = {
+ DEFINE_RES_IRQ_NAMED(MT6360_ADC_DONEI, "adc_donei"),
+};
+
+static const struct resource mt6360_chg_resources[] = {
+ DEFINE_RES_IRQ_NAMED(MT6360_CHG_TREG_EVT, "chg_treg_evt"),
+ DEFINE_RES_IRQ_NAMED(MT6360_PWR_RDY_EVT, "pwr_rdy_evt"),
+ DEFINE_RES_IRQ_NAMED(MT6360_CHG_BATSYSUV_EVT, "chg_batsysuv_evt"),
+ DEFINE_RES_IRQ_NAMED(MT6360_CHG_VSYSUV_EVT, "chg_vsysuv_evt"),
+ DEFINE_RES_IRQ_NAMED(MT6360_CHG_VSYSOV_EVT, "chg_vsysov_evt"),
+ DEFINE_RES_IRQ_NAMED(MT6360_CHG_VBATOV_EVT, "chg_vbatov_evt"),
+ DEFINE_RES_IRQ_NAMED(MT6360_CHG_VBUSOV_EVT, "chg_vbusov_evt"),
+ DEFINE_RES_IRQ_NAMED(MT6360_CHG_AICCMEASL, "chg_aiccmeasl"),
+ DEFINE_RES_IRQ_NAMED(MT6360_WDTMRI, "wdtmri"),
+ DEFINE_RES_IRQ_NAMED(MT6360_CHG_RECHGI, "chg_rechgi"),
+ DEFINE_RES_IRQ_NAMED(MT6360_CHG_TERMI, "chg_termi"),
+ DEFINE_RES_IRQ_NAMED(MT6360_CHG_IEOCI, "chg_ieoci"),
+ DEFINE_RES_IRQ_NAMED(MT6360_PUMPX_DONEI, "pumpx_donei"),
+ DEFINE_RES_IRQ_NAMED(MT6360_ATTACH_I, "attach_i"),
+ DEFINE_RES_IRQ_NAMED(MT6360_CHRDET_EXT_EVT, "chrdet_ext_evt"),
+};
+
+static const struct resource mt6360_led_resources[] = {
+ DEFINE_RES_IRQ_NAMED(MT6360_FLED_CHG_VINOVP_EVT, "fled_chg_vinovp_evt"),
+ DEFINE_RES_IRQ_NAMED(MT6360_FLED_LVF_EVT, "fled_lvf_evt"),
+ DEFINE_RES_IRQ_NAMED(MT6360_FLED2_SHORT_EVT, "fled2_short_evt"),
+ DEFINE_RES_IRQ_NAMED(MT6360_FLED1_SHORT_EVT, "fled1_short_evt"),
+ DEFINE_RES_IRQ_NAMED(MT6360_FLED2_STRB_TO_EVT, "fled2_strb_to_evt"),
+ DEFINE_RES_IRQ_NAMED(MT6360_FLED1_STRB_TO_EVT, "fled1_strb_to_evt"),
+};
+
+static const struct resource mt6360_pmic_resources[] = {
+ DEFINE_RES_IRQ_NAMED(MT6360_BUCK1_PGB_EVT, "buck1_pgb_evt"),
+ DEFINE_RES_IRQ_NAMED(MT6360_BUCK1_OC_EVT, "buck1_oc_evt"),
+ DEFINE_RES_IRQ_NAMED(MT6360_BUCK1_OV_EVT, "buck1_ov_evt"),
+ DEFINE_RES_IRQ_NAMED(MT6360_BUCK1_UV_EVT, "buck1_uv_evt"),
+ DEFINE_RES_IRQ_NAMED(MT6360_BUCK2_PGB_EVT, "buck2_pgb_evt"),
+ DEFINE_RES_IRQ_NAMED(MT6360_BUCK2_OC_EVT, "buck2_oc_evt"),
+ DEFINE_RES_IRQ_NAMED(MT6360_BUCK2_OV_EVT, "buck2_ov_evt"),
+ DEFINE_RES_IRQ_NAMED(MT6360_BUCK2_UV_EVT, "buck2_uv_evt"),
+ DEFINE_RES_IRQ_NAMED(MT6360_LDO6_OC_EVT, "ldo6_oc_evt"),
+ DEFINE_RES_IRQ_NAMED(MT6360_LDO7_OC_EVT, "ldo7_oc_evt"),
+ DEFINE_RES_IRQ_NAMED(MT6360_LDO6_PGB_EVT, "ldo6_pgb_evt"),
+ DEFINE_RES_IRQ_NAMED(MT6360_LDO7_PGB_EVT, "ldo7_pgb_evt"),
+};
+
+static const struct resource mt6360_ldo_resources[] = {
+ DEFINE_RES_IRQ_NAMED(MT6360_LDO1_OC_EVT, "ldo1_oc_evt"),
+ DEFINE_RES_IRQ_NAMED(MT6360_LDO2_OC_EVT, "ldo2_oc_evt"),
+ DEFINE_RES_IRQ_NAMED(MT6360_LDO3_OC_EVT, "ldo3_oc_evt"),
+ DEFINE_RES_IRQ_NAMED(MT6360_LDO5_OC_EVT, "ldo5_oc_evt"),
+ DEFINE_RES_IRQ_NAMED(MT6360_LDO1_PGB_EVT, "ldo1_pgb_evt"),
+ DEFINE_RES_IRQ_NAMED(MT6360_LDO2_PGB_EVT, "ldo2_pgb_evt"),
+ DEFINE_RES_IRQ_NAMED(MT6360_LDO3_PGB_EVT, "ldo3_pgb_evt"),
+ DEFINE_RES_IRQ_NAMED(MT6360_LDO5_PGB_EVT, "ldo5_pgb_evt"),
+};
+
+static const struct mfd_cell mt6360_devs[] = {
+ MT6360_MFD_CELL(mt6360_adc),
+ MT6360_MFD_CELL(mt6360_chg),
+ MT6360_MFD_CELL(mt6360_led),
+ MT6360_MFD_CELL(mt6360_pmic),
+ MT6360_MFD_CELL(mt6360_ldo),
+ /* tcpc dev */
+ {
+ .name = "mt6360_tcpc",
+ .of_compatible = "mediatek,mt6360_tcpc",
+ },
+};
+
+static const unsigned short mt6360_slave_addr[MT6360_SLAVE_MAX] = {
+ MT6360_PMU_SLAVEID,
+ MT6360_PMIC_SLAVEID,
+ MT6360_LDO_SLAVEID,
+ MT6360_TCPC_SLAVEID,
+};
+
+static int mt6360_pmu_probe(struct i2c_client *client,
+ const struct i2c_device_id *id)
+{
+ struct mt6360_pmu_info *mpi;
+ unsigned int reg_data = 0;
+ int i, ret;
+
+ mpi = devm_kzalloc(&client->dev, sizeof(*mpi), GFP_KERNEL);
+ if (!mpi)
+ return -ENOMEM;
+ mpi->dev = &client->dev;
+ i2c_set_clientdata(client, mpi);
+
+ /* regmap regiser */
+ mpi->regmap = devm_regmap_init_i2c(client, &mt6360_pmu_regmap_config);
+ if (IS_ERR(mpi->regmap)) {
+ dev_err(&client->dev, "regmap register fail\n");
+ return PTR_ERR(mpi->regmap);
+ }
+ /* chip id check */
+ ret = regmap_read(mpi->regmap, MT6360_PMU_DEV_INFO, ®_data);
+ if (ret < 0) {
+ dev_err(&client->dev, "device not found\n");
+ return ret;
+ }
+ if ((reg_data & CHIP_VEN_MASK) != CHIP_VEN_MT6360) {
+ dev_err(&client->dev, "not mt6360 chip\n");
+ return -ENODEV;
+ }
+ mpi->chip_rev = reg_data & CHIP_REV_MASK;
+ /* irq register */
+ memcpy(&mpi->irq_chip, &mt6360_pmu_irq_chip, sizeof(mpi->irq_chip));
+ mpi->irq_chip.name = dev_name(&client->dev);
+ mpi->irq_chip.irq_drv_data = mpi;
+ ret = devm_regmap_add_irq_chip(&client->dev, mpi->regmap, client->irq,
+ IRQF_TRIGGER_FALLING, 0, &mpi->irq_chip,
+ &mpi->irq_data);
+ if (ret < 0) {
+ dev_err(&client->dev, "regmap irq chip add fail\n");
+ return ret;
+ }
+ /* new i2c slave device */
+ for (i = 0; i < MT6360_SLAVE_MAX; i++) {
+ if (mt6360_slave_addr[i] == client->addr) {
+ mpi->i2c[i] = client;
+ continue;
+ }
+ mpi->i2c[i] = i2c_new_dummy(client->adapter,
+ mt6360_slave_addr[i]);
+ if (!mpi->i2c[i]) {
+ dev_err(&client->dev, "new i2c dev [%d] fail\n", i);
+ ret = -ENODEV;
+ goto out;
+ }
+ i2c_set_clientdata(mpi->i2c[i], mpi);
+ }
+ /* mfd cell register */
+ ret = devm_mfd_add_devices(&client->dev, PLATFORM_DEVID_AUTO,
+ mt6360_devs, ARRAY_SIZE(mt6360_devs), NULL,
+ 0, regmap_irq_get_domain(mpi->irq_data));
+ if (ret < 0) {
+ dev_err(&client->dev, "mfd add cells fail\n");
+ goto out;
+ }
+ dev_info(&client->dev, "Successfully probed\n");
+ return 0;
+out:
+ while (--i >= 0) {
+ if (mpi->i2c[i]->addr == client->addr)
+ continue;
+ i2c_unregister_device(mpi->i2c[i]);
+ }
+ return ret;
+}
+
+static int mt6360_pmu_remove(struct i2c_client *client)
+{
+ struct mt6360_pmu_info *mpi = i2c_get_clientdata(client);
+ int i;
+
+ for (i = 0; i < MT6360_SLAVE_MAX; i++) {
+ if (mpi->i2c[i]->addr == client->addr)
+ continue;
+ i2c_unregister_device(mpi->i2c[i]);
+ }
+ return 0;
+}
+
+static int __maybe_unused mt6360_pmu_suspend(struct device *dev)
+{
+ struct i2c_client *i2c = to_i2c_client(dev);
+
+ if (device_may_wakeup(dev))
+ enable_irq_wake(i2c->irq);
+ return 0;
+}
+
+static int __maybe_unused mt6360_pmu_resume(struct device *dev)
+{
+
+ struct i2c_client *i2c = to_i2c_client(dev);
+
+ if (device_may_wakeup(dev))
+ disable_irq_wake(i2c->irq);
+ return 0;
+}
+
+static SIMPLE_DEV_PM_OPS(mt6360_pmu_pm_ops,
+ mt6360_pmu_suspend, mt6360_pmu_resume);
+
+static const struct of_device_id __maybe_unused mt6360_pmu_of_id[] = {
+ { .compatible = "mediatek,mt6360_pmu", },
+ {},
+};
+MODULE_DEVICE_TABLE(of, mt6360_pmu_of_id);
+
+static const struct i2c_device_id mt6360_pmu_id[] = {
+ { "mt6360_pmu", 0 },
+ {},
+};
+MODULE_DEVICE_TABLE(i2c, mt6360_pmu_id);
+
+static struct i2c_driver mt6360_pmu_driver = {
+ .driver = {
+ .name = "mt6360_pmu",
+ .owner = THIS_MODULE,
+ .pm = &mt6360_pmu_pm_ops,
+ .of_match_table = of_match_ptr(mt6360_pmu_of_id),
+ },
+ .probe = mt6360_pmu_probe,
+ .remove = mt6360_pmu_remove,
+ .id_table = mt6360_pmu_id,
+};
+module_i2c_driver(mt6360_pmu_driver);
+
+MODULE_AUTHOR("CY_Huang <cy_huang@richtek.com>");
+MODULE_DESCRIPTION("MT6360 PMU I2C Driver");
+MODULE_LICENSE("GPL");
+MODULE_VERSION("1.0.0");
--
1.9.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [PATCH] ASoC: atmel_ssc_dai: Remove wrong spinlock usage
From: Alexandre Belloni @ 2019-09-18 9:48 UTC (permalink / raw)
To: Gregory CLEMENT
Cc: alsa-devel, linux-kernel, Liam Girdwood, Ludovic Desroches,
Mark Brown, Codrin Ciubotariu, linux-arm-kernel
In-Reply-To: <20190918094114.15867-1-gregory.clement@bootlin.com>
On 18/09/2019 11:41:14+0200, Gregory CLEMENT wrote:
> A potential bug was reported in the email "[BUG] atmel_ssc_dai: a
> possible sleep-in-atomic bug in atmel_ssc_shutdown"[1]
>
> Indeed in the function atmel_ssc_shutdown() free_irq() was called in a
> critical section protected by spinlock.
>
> However this spinlock is only used in atmel_ssc_shutdown() and
> atmel_ssc_startup() functions. After further analysis, it occurred that
> the call to these function are already protected by mutex used on the
> calling functions.
>
> Then we can remove the spinlock which will fix this bug as a side
> effect. Thanks to this patch the following message disappears:
>
> "BUG: sleeping function called from invalid context at
> kernel/locking/mutex.c:909"
>
> [1]: https://www.spinics.net/lists/alsa-devel/msg71286.html
>
> Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com>
> ---
> sound/soc/atmel/atmel_ssc_dai.c | 7 -------
> 1 file changed, 7 deletions(-)
>
> diff --git a/sound/soc/atmel/atmel_ssc_dai.c b/sound/soc/atmel/atmel_ssc_dai.c
> index 6f89483ac88c..365957e86419 100644
> --- a/sound/soc/atmel/atmel_ssc_dai.c
> +++ b/sound/soc/atmel/atmel_ssc_dai.c
> @@ -116,19 +116,16 @@ static struct atmel_pcm_dma_params ssc_dma_params[NUM_SSC_DEVICES][2] = {
> static struct atmel_ssc_info ssc_info[NUM_SSC_DEVICES] = {
> {
> .name = "ssc0",
> - .lock = __SPIN_LOCK_UNLOCKED(ssc_info[0].lock),
This member is now unused and can be removed from the struct.
Once fixed, Reviewed-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
> .dir_mask = SSC_DIR_MASK_UNUSED,
> .initialized = 0,
> },
> {
> .name = "ssc1",
> - .lock = __SPIN_LOCK_UNLOCKED(ssc_info[1].lock),
> .dir_mask = SSC_DIR_MASK_UNUSED,
> .initialized = 0,
> },
> {
> .name = "ssc2",
> - .lock = __SPIN_LOCK_UNLOCKED(ssc_info[2].lock),
> .dir_mask = SSC_DIR_MASK_UNUSED,
> .initialized = 0,
> },
> @@ -317,13 +314,11 @@ static int atmel_ssc_startup(struct snd_pcm_substream *substream,
>
> snd_soc_dai_set_dma_data(dai, substream, dma_params);
>
> - spin_lock_irq(&ssc_p->lock);
> if (ssc_p->dir_mask & dir_mask) {
> spin_unlock_irq(&ssc_p->lock);
> return -EBUSY;
> }
> ssc_p->dir_mask |= dir_mask;
> - spin_unlock_irq(&ssc_p->lock);
>
> return 0;
> }
> @@ -355,7 +350,6 @@ static void atmel_ssc_shutdown(struct snd_pcm_substream *substream,
>
> dir_mask = 1 << dir;
>
> - spin_lock_irq(&ssc_p->lock);
> ssc_p->dir_mask &= ~dir_mask;
> if (!ssc_p->dir_mask) {
> if (ssc_p->initialized) {
> @@ -369,7 +363,6 @@ static void atmel_ssc_shutdown(struct snd_pcm_substream *substream,
> ssc_p->cmr_div = ssc_p->tcmr_period = ssc_p->rcmr_period = 0;
> ssc_p->forced_divider = 0;
> }
> - spin_unlock_irq(&ssc_p->lock);
>
> /* Shutdown the SSC clock. */
> pr_debug("atmel_ssc_dai: Stopping clock\n");
> --
> 2.23.0
>
--
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH V6 1/2] dt-bindings: mailbox: add binding doc for the ARM SMC/HVC mailbox
From: Andre Przywara @ 2019-09-18 9:43 UTC (permalink / raw)
To: Jassi Brar
Cc: mark.rutland@arm.com, devicetree@vger.kernel.org, Peng Fan,
f.fainelli@gmail.com, linux-kernel@vger.kernel.org,
robh+dt@kernel.org, dl-linux-imx, sudeep.holla@arm.com,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <CABb+yY2CP1i9fZMoPua=-mLCUpYrcO28xF5UXDeRD2XTYe7mEg@mail.gmail.com>
On Wed, 18 Sep 2019 00:27:00 -0500
Jassi Brar <jassisinghbrar@gmail.com> wrote:
Hi,
> On Tue, Sep 17, 2019 at 12:31 PM Andre Przywara <andre.przywara@arm.com> wrote:
> >
> > On Mon, 16 Sep 2019 09:44:37 +0000
> > Peng Fan <peng.fan@nxp.com> wrote:
> >
> > Hi,
> >
> > > From: Peng Fan <peng.fan@nxp.com>
> > >
> > > The ARM SMC/HVC mailbox binding describes a firmware interface to trigger
> > > actions in software layers running in the EL2 or EL3 exception levels.
> > > The term "ARM" here relates to the SMC instruction as part of the ARM
> > > instruction set, not as a standard endorsed by ARM Ltd.
> > >
> > > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > > ---
> > > .../devicetree/bindings/mailbox/arm-smc.yaml | 96 ++++++++++++++++++++++
> > > 1 file changed, 96 insertions(+)
> > > create mode 100644 Documentation/devicetree/bindings/mailbox/arm-smc.yaml
> > >
> > > diff --git a/Documentation/devicetree/bindings/mailbox/arm-smc.yaml b/Documentation/devicetree/bindings/mailbox/arm-smc.yaml
> > > new file mode 100644
> > > index 000000000000..bf01bec035fc
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/mailbox/arm-smc.yaml
> > > @@ -0,0 +1,96 @@
> > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/mailbox/arm-smc.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: ARM SMC Mailbox Interface
> > > +
> > > +maintainers:
> > > + - Peng Fan <peng.fan@nxp.com>
> > > +
> > > +description: |
> > > + This mailbox uses the ARM smc (secure monitor call) and hvc (hypervisor
> >
> > I think "or" instead of "and" is less confusing.
> >
> > > + call) instruction to trigger a mailbox-connected activity in firmware,
> > > + executing on the very same core as the caller. The value of r0/w0/x0
> > > + the firmware returns after the smc call is delivered as a received
> > > + message to the mailbox framework, so synchronous communication can be
> > > + established. The exact meaning of the action the mailbox triggers as
> > > + well as the return value is defined by their users and is not subject
> > > + to this binding.
> > > +
> > > + One use case of this mailbox is the SCMI interface, which uses shared
> >
> > One example use case of this mailbox ...
> > (to make it more obvious that it's not restricted to this)
> >
> > > + memory to transfer commands and parameters, and a mailbox to trigger a
> > > + function call. This allows SoCs without a separate management processor
> > > + (or when such a processor is not available or used) to use this
> > > + standardized interface anyway.
> > > +
> > > + This binding describes no hardware, but establishes a firmware interface.
> > > + Upon receiving an SMC using one of the described SMC function identifiers,
> >
> > ... the described SMC function identifier,
> >
> > > + the firmware is expected to trigger some mailbox connected functionality.
> > > + The communication follows the ARM SMC calling convention.
> > > + Firmware expects an SMC function identifier in r0 or w0. The supported
> > > + identifiers are passed from consumers,
> >
> > identifier
> >
> > "passed from consumers": How? Where?
> > But I want to repeat: We should not allow this.
> > This is a binding for a mailbox controller driver, not a generic firmware backdoor.
> >
> Exactly. The mailbox controller here is the SMC/HVC instruction,
No, the mailbox controller is an *SMCCC compliant* smc/hvc call, targeting a very specific function ID. SMC calls are used for PSCI already, for instance, and you don't want to mess with that. Also some platforms define a vendor specific smc interface, again using a well constructed function ID complying to SMCCC.
So we definitely need to stay within SMCCC for this kind of generic interface, *and* to let firmware specify the function ID via the DT, to not clash with any other function ID.
> which needs 9 arguments to work. The fact that the fist argument is
> always going to be same on a platform is just the way we use this
> instruction.
>
> > We should be as strict as possible to avoid any security issues.
> >
> Any example of such a security issue?
Someone finds a way to trick some mailbox client to send a crafted message to the mailbox.
Do you have any example of a use case where the mailbox client needs to provide the function ID?
> > The firmware certainly knows the function ID it implements. The firmware controls the DT. So it is straight-forward to put the ID into the DT. The firmware could even do this at boot time, dynamically, before passing on the DT to the non-secure world (bootloader or kernel).
> >
> > What would be the use case of this functionality?
> >
> At least for flexibility and consistency.
I appreciate the flexibility idea, but when creating an interface, especially a generic one to any kind of firmware, you should be as strict as possible, to avoid clashes in the future.
> > > or listed in the the arm,func-ids
> >
> > arm,func-id
> >
> > > + properties as described below. The firmware can return one value in
> >
> > property
> >
> > > + the first SMC result register, it is expected to be an error value,
> > > + which shall be propagated to the mailbox client.
> > > +
> > > + Any core which supports the SMC or HVC instruction can be used, as long
> > > + as a firmware component running in EL3 or EL2 is handling these calls.
> > > +
> > > +properties:
> > > + compatible:
> > > + oneOf:
> > > + - description:
> > > + For implementations using ARM SMC instruction.
> > > + const: arm,smc-mbox
> > > +
> > > + - description:
> > > + For implementations using ARM HVC instruction.
> > > + const: arm,hvc-mbox
> >
> > I am not particularly happy with this, but well ...
> >
> > > +
> > > + "#mbox-cells":
> > > + const: 1
> >
> > Why is this "1"? What is this number used for? It used to be the channel ID, but since you are describing a single channel controller only, this should be 0 now.
> >
> Yes. I overlooked it and actually queued the patch for pull request.
> But I think the bindings should not carry a 'fix' patch later. Also I
> realise this revision of binding hasn't been reviewed by Rob. Maybe I
> should drop the patch for now.
Yes, please do. I would like to make sure that the binding is correct, as it serves as a specification for people implementing both firmware services and other drivers (like *BSD).
> > > +
> > > + arm,func-id:
> > > + description: |
> > > + An 32-bit value specifying the function ID used by the mailbox.
> >
> > A single 32-bit value ...
> >
> > > + The function ID follow the ARM SMC calling convention standard [1].
> >
> > follows
> >
> > > + $ref: /schemas/types.yaml#/definitions/uint32
> > > +
> > > +required:
> > > + - compatible
> > > + - "#mbox-cells"
> > > +
> > > +examples:
> > > + - |
> > > + sram@93f000 {
> > > + compatible = "mmio-sram";
> > > + reg = <0x0 0x93f000 0x0 0x1000>;
> > > + #address-cells = <1>;
> > > + #size-cells = <1>;
> > > + ranges = <0x0 0x93f000 0x1000>;
> > > +
> > > + cpu_scp_lpri: scp-shmem@0 {
> > > + compatible = "arm,scmi-shmem";
> > > + reg = <0x0 0x200>;
> > > + };
> > > + };
> > > +
> > > + smc_tx_mbox: tx_mbox {
> > > + #mbox-cells = <1>;
> >
> > As mentioned above, should be 0.
> >
> > > + compatible = "arm,smc-mbox";
> > > + /* optional */
> >
> > First: having "optional" in a specific example is not helpful, just confusing.
> > Second: It is actually *not* optional in this case, as there is no other way of propagating the function ID. The SCMI driver as the mailbox client has certainly no clue about this.
> > I think I said this previously: Relying on the mailbox client to pass the function ID sounds broken, as this is a property of the mailbox controller driver. The mailbox client does not care about this mailbox communication detail, it just wants to trigger the mailbox.
> >
> Again, the mailbox controller here is the SMC/HVC _instruction_, which
> doesn't care what value the first argument carry.
That is not true. Just check Peng's example implementation he mentioned in the cover letter:
#define FSL_SIP_SCMI_1 0xc20000fe
#define FSL_SIP_SCMI_2 0xc20000ff
....
case FSL_SIP_SCMI_1:
case FSL_SIP_SCMI_2:
SMC_RET1(handle, scmi_handler(smc_fid, x1, x2, x3));
Definitely the function ID is crucial here.
Cheers,
Andre.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH] ASoC: atmel_ssc_dai: Remove wrong spinlock usage
From: Gregory CLEMENT @ 2019-09-18 9:41 UTC (permalink / raw)
To: Codrin Ciubotariu, Liam Girdwood, Mark Brown, alsa-devel,
linux-kernel
Cc: Alexandre Belloni, Ludovic Desroches, linux-arm-kernel,
Gregory CLEMENT
A potential bug was reported in the email "[BUG] atmel_ssc_dai: a
possible sleep-in-atomic bug in atmel_ssc_shutdown"[1]
Indeed in the function atmel_ssc_shutdown() free_irq() was called in a
critical section protected by spinlock.
However this spinlock is only used in atmel_ssc_shutdown() and
atmel_ssc_startup() functions. After further analysis, it occurred that
the call to these function are already protected by mutex used on the
calling functions.
Then we can remove the spinlock which will fix this bug as a side
effect. Thanks to this patch the following message disappears:
"BUG: sleeping function called from invalid context at
kernel/locking/mutex.c:909"
[1]: https://www.spinics.net/lists/alsa-devel/msg71286.html
Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com>
---
sound/soc/atmel/atmel_ssc_dai.c | 7 -------
1 file changed, 7 deletions(-)
diff --git a/sound/soc/atmel/atmel_ssc_dai.c b/sound/soc/atmel/atmel_ssc_dai.c
index 6f89483ac88c..365957e86419 100644
--- a/sound/soc/atmel/atmel_ssc_dai.c
+++ b/sound/soc/atmel/atmel_ssc_dai.c
@@ -116,19 +116,16 @@ static struct atmel_pcm_dma_params ssc_dma_params[NUM_SSC_DEVICES][2] = {
static struct atmel_ssc_info ssc_info[NUM_SSC_DEVICES] = {
{
.name = "ssc0",
- .lock = __SPIN_LOCK_UNLOCKED(ssc_info[0].lock),
.dir_mask = SSC_DIR_MASK_UNUSED,
.initialized = 0,
},
{
.name = "ssc1",
- .lock = __SPIN_LOCK_UNLOCKED(ssc_info[1].lock),
.dir_mask = SSC_DIR_MASK_UNUSED,
.initialized = 0,
},
{
.name = "ssc2",
- .lock = __SPIN_LOCK_UNLOCKED(ssc_info[2].lock),
.dir_mask = SSC_DIR_MASK_UNUSED,
.initialized = 0,
},
@@ -317,13 +314,11 @@ static int atmel_ssc_startup(struct snd_pcm_substream *substream,
snd_soc_dai_set_dma_data(dai, substream, dma_params);
- spin_lock_irq(&ssc_p->lock);
if (ssc_p->dir_mask & dir_mask) {
spin_unlock_irq(&ssc_p->lock);
return -EBUSY;
}
ssc_p->dir_mask |= dir_mask;
- spin_unlock_irq(&ssc_p->lock);
return 0;
}
@@ -355,7 +350,6 @@ static void atmel_ssc_shutdown(struct snd_pcm_substream *substream,
dir_mask = 1 << dir;
- spin_lock_irq(&ssc_p->lock);
ssc_p->dir_mask &= ~dir_mask;
if (!ssc_p->dir_mask) {
if (ssc_p->initialized) {
@@ -369,7 +363,6 @@ static void atmel_ssc_shutdown(struct snd_pcm_substream *substream,
ssc_p->cmr_div = ssc_p->tcmr_period = ssc_p->rcmr_period = 0;
ssc_p->forced_divider = 0;
}
- spin_unlock_irq(&ssc_p->lock);
/* Shutdown the SSC clock. */
pr_debug("atmel_ssc_dai: Stopping clock\n");
--
2.23.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [PATCH V6 0/3] mmc: mmci: add busy detect for stm32 sdmmc variant
From: Ludovic BARRE @ 2019-09-18 9:33 UTC (permalink / raw)
To: Ulf Hansson, Rob Herring
Cc: devicetree, Alexandre Torgue, linux-mmc, linux-kernel,
srinivas.kandagatla, Maxime Coquelin, linux-stm32,
linux-arm-kernel
In-Reply-To: <20190905122112.29672-1-ludovic.Barre@st.com>
hi Ulf
Just a "gentleman ping" about this series and
https://lkml.org/lkml/2019/9/4/747
Regards
Ludo
Le 9/5/19 à 2:21 PM, Ludovic Barre a écrit :
> From: Ludovic Barre <ludovic.barre@st.com>
>
> This patch series adds busy detect for stm32 sdmmc variant.
> Some adaptations are required:
> -On sdmmc the data timer is started on data transfert
> and busy state, so we must add hardware busy timeout support.
> -Add busy_complete callback at mmci_host_ops to allow to define
> a specific busy completion by variant.
> -Add sdmmc busy_complete callback.
>
> V6:
> -mmci_start_command: set datatimer only on rsp_busy flag
> (remove host->mrq->data).
> -move max_busy_timeout in set_ios callback.
> -typo fix: err_msk, clks on one lines.
>
> V5:
> -Replaces !cmd->data to !host->mrq->data to avoid overwrite
> of datatimer register by the first command (cmd23, without data) of
> SBC request.
>
> V4:
> -Re-work with busy_complete callback
> -In series, move "mmc: mmci: add hardware busy timeout feature" in
> first to simplify busy_complete prototype with err_msk parameter.
>
> V3:
> -rebase on latest mmc next
> -replace re-read by status parameter.
>
> V2:
> -mmci_cmd_irq cleanup in separate patch.
> -simplify the busy_detect_flag exclude
> -replace sdmmc specific comment in
> "mmc: mmci: avoid fake busy polling in mmci_irq"
> to focus on common behavior
>
> Ludovic Barre (3):
> mmc: mmci: add hardware busy timeout feature
> mmc: mmci: add busy_complete callback
> mmc: mmci: sdmmc: add busy_complete callback
>
> drivers/mmc/host/mmci.c | 183 +++++++++++++++++-----------
> drivers/mmc/host/mmci.h | 7 +-
> drivers/mmc/host/mmci_stm32_sdmmc.c | 38 ++++++
> 3 files changed, 156 insertions(+), 72 deletions(-)
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH] media: platform: Use devm_platform_ioremap_resource() in two functions
From: Markus Elfring @ 2019-09-18 9:30 UTC (permalink / raw)
To: linux-media, linux-arm-kernel, linux-mediatek, linux-renesas-soc,
Andrew-CT Chen, Matthias Brugger, Mauro Carvalho Chehab,
Niklas Söderlund, Tiffany Lin
Cc: Bartosz Golaszewski, kernel-janitors, LKML, Himanshu Jha
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 18 Sep 2019 11:20:48 +0200
Simplify these function implementations by using a known wrapper function.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_drv.c | 8 +-------
drivers/media/platform/rcar-vin/rcar-core.c | 7 +------
2 files changed, 2 insertions(+), 13 deletions(-)
diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_drv.c b/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_drv.c
index 00d090df11bb..944771ee5f5c 100644
--- a/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_drv.c
+++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_drv.c
@@ -253,13 +253,7 @@ static int mtk_vcodec_probe(struct platform_device *pdev)
}
for (i = 0; i < NUM_MAX_VDEC_REG_BASE; i++) {
- res = platform_get_resource(pdev, IORESOURCE_MEM, i);
- if (res == NULL) {
- dev_err(&pdev->dev, "get memory resource failed.");
- ret = -ENXIO;
- goto err_res;
- }
- dev->reg_base[i] = devm_ioremap_resource(&pdev->dev, res);
+ dev->reg_base[i] = devm_platform_ioremap_resource(pdev, i);
if (IS_ERR((__force void *)dev->reg_base[i])) {
ret = PTR_ERR((__force void *)dev->reg_base[i]);
goto err_res;
diff --git a/drivers/media/platform/rcar-vin/rcar-core.c b/drivers/media/platform/rcar-vin/rcar-core.c
index 6993484ff0f3..334c62805959 100644
--- a/drivers/media/platform/rcar-vin/rcar-core.c
+++ b/drivers/media/platform/rcar-vin/rcar-core.c
@@ -1282,7 +1282,6 @@ static int rcar_vin_probe(struct platform_device *pdev)
{
const struct soc_device_attribute *attr;
struct rvin_dev *vin;
- struct resource *mem;
int irq, ret;
vin = devm_kzalloc(&pdev->dev, sizeof(*vin), GFP_KERNEL);
@@ -1301,11 +1300,7 @@ static int rcar_vin_probe(struct platform_device *pdev)
if (attr)
vin->info = attr->data;
- mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- if (mem == NULL)
- return -EINVAL;
-
- vin->base = devm_ioremap_resource(vin->dev, mem);
+ vin->base = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(vin->base))
return PTR_ERR(vin->base);
--
2.23.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [PATCH V7 1/3] mm/hotplug: Reorder memblock_[free|remove]() calls in try_remove_memory()
From: Anshuman Khandual @ 2019-09-18 9:28 UTC (permalink / raw)
To: Balbir Singh, linux-mm, linux-kernel, linux-arm-kernel, akpm,
catalin.marinas, will
Cc: mark.rutland, mhocko, david, ira.weiny, steve.capper, mgorman,
steven.price, broonie, cai, ard.biesheuvel, cpandya, arunks,
dan.j.williams, Robin.Murphy, logang, valentin.schneider,
suzuki.poulose, osalvador
In-Reply-To: <74bcbd36-3bec-be67-917d-60cd74cbcef0@gmail.com>
On 09/16/2019 07:14 AM, Balbir Singh wrote:
>
>
> On 3/9/19 7:45 pm, Anshuman Khandual wrote:
>> Memory hot remove uses get_nid_for_pfn() while tearing down linked sysfs
>
> I could not find this path in the code, the only called for get_nid_for_pfn()
> was register_mem_sect_under_node() when the system is under boot.
>
>> entries between memory block and node. It first checks pfn validity with
>> pfn_valid_within() before fetching nid. With CONFIG_HOLES_IN_ZONE config
>> (arm64 has this enabled) pfn_valid_within() calls pfn_valid().
>>
>> pfn_valid() is an arch implementation on arm64 (CONFIG_HAVE_ARCH_PFN_VALID)
>> which scans all mapped memblock regions with memblock_is_map_memory(). This
>> creates a problem in memory hot remove path which has already removed given
>> memory range from memory block with memblock_[remove|free] before arriving
>> at unregister_mem_sect_under_nodes(). Hence get_nid_for_pfn() returns -1
>> skipping subsequent sysfs_remove_link() calls leaving node <-> memory block
>> sysfs entries as is. Subsequent memory add operation hits BUG_ON() because
>> of existing sysfs entries.
>>
>> [ 62.007176] NUMA: Unknown node for memory at 0x680000000, assuming node 0
>> [ 62.052517] ------------[ cut here ]------------
>
> This seems like arm64 is not ready for probe_store() via drivers/base/memory.c/ode.c
>
>> [ 62.053211] kernel BUG at mm/memory_hotplug.c:1143!
>
>
>
>> [ 62.053868] Internal error: Oops - BUG: 0 [#1] PREEMPT SMP
>> [ 62.054589] Modules linked in:
>> [ 62.054999] CPU: 19 PID: 3275 Comm: bash Not tainted 5.1.0-rc2-00004-g28cea40b2683 #41
>> [ 62.056274] Hardware name: linux,dummy-virt (DT)
>> [ 62.057166] pstate: 40400005 (nZcv daif +PAN -UAO)
>> [ 62.058083] pc : add_memory_resource+0x1cc/0x1d8
>> [ 62.058961] lr : add_memory_resource+0x10c/0x1d8
>> [ 62.059842] sp : ffff0000168b3ce0
>> [ 62.060477] x29: ffff0000168b3ce0 x28: ffff8005db546c00
>> [ 62.061501] x27: 0000000000000000 x26: 0000000000000000
>> [ 62.062509] x25: ffff0000111ef000 x24: ffff0000111ef5d0
>> [ 62.063520] x23: 0000000000000000 x22: 00000006bfffffff
>> [ 62.064540] x21: 00000000ffffffef x20: 00000000006c0000
>> [ 62.065558] x19: 0000000000680000 x18: 0000000000000024
>> [ 62.066566] x17: 0000000000000000 x16: 0000000000000000
>> [ 62.067579] x15: ffffffffffffffff x14: ffff8005e412e890
>> [ 62.068588] x13: ffff8005d6b105d8 x12: 0000000000000000
>> [ 62.069610] x11: ffff8005d6b10490 x10: 0000000000000040
>> [ 62.070615] x9 : ffff8005e412e898 x8 : ffff8005e412e890
>> [ 62.071631] x7 : ffff8005d6b105d8 x6 : ffff8005db546c00
>> [ 62.072640] x5 : 0000000000000001 x4 : 0000000000000002
>> [ 62.073654] x3 : ffff8005d7049480 x2 : 0000000000000002
>> [ 62.074666] x1 : 0000000000000003 x0 : 00000000ffffffef
>> [ 62.075685] Process bash (pid: 3275, stack limit = 0x00000000d754280f)
>> [ 62.076930] Call trace:
>> [ 62.077411] add_memory_resource+0x1cc/0x1d8
>> [ 62.078227] __add_memory+0x70/0xa8
>> [ 62.078901] probe_store+0xa4/0xc8
>> [ 62.079561] dev_attr_store+0x18/0x28
>> [ 62.080270] sysfs_kf_write+0x40/0x58
>> [ 62.080992] kernfs_fop_write+0xcc/0x1d8
>> [ 62.081744] __vfs_write+0x18/0x40
>> [ 62.082400] vfs_write+0xa4/0x1b0
>> [ 62.083037] ksys_write+0x5c/0xc0
>> [ 62.083681] __arm64_sys_write+0x18/0x20
>> [ 62.084432] el0_svc_handler+0x88/0x100
>> [ 62.085177] el0_svc+0x8/0xc
>>
>> Re-ordering memblock_[free|remove]() with arch_remove_memory() solves the
>> problem on arm64 as pfn_valid() behaves correctly and returns positive
>> as memblock for the address range still exists. arch_remove_memory()
>> removes applicable memory sections from zone with __remove_pages() and
>> tears down kernel linear mapping. Removing memblock regions afterwards
>> is safe because there is no other memblock (bootmem) allocator user that
>> late. So nobody is going to allocate from the removed range just to blow
>> up later. Also nobody should be using the bootmem allocated range else
>> we wouldn't allow to remove it. So reordering is indeed safe.
>>
>> Reviewed-by: David Hildenbrand <david@redhat.com>
>> Reviewed-by: Oscar Salvador <osalvador@suse.de>
>> Acked-by: Mark Rutland <mark.rutland@arm.com>
>> Acked-by: Michal Hocko <mhocko@suse.com>
>> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
>> ---
>
> Honestly, the issue is not clear from the changelog, largely
> because I can't find the use of get_nid_for_pfn() being used
> in memory hotunplug. I can see why using pfn_valid() after
> memblock_free/remove is bad on the architecture.
>
> I think the checks to pfn_valid() can be avoided from the
> remove paths if we did the following
>
> memblock_isolate_regions()
> for each isolate_region {
> memblock_free
> memblock_remove
> arch_memory_remove
>
> # ensure that __remove_memory can avoid calling pfn_valid
> }
>
> Having said that, your patch is easier and if your assumption
> about not using the memblocks is valid (after arch_memory_remove())
> then might be the least resistant way forward
The context for this patch has changed a bit which now reflects in
it's current posting (https://patchwork.kernel.org/patch/11146361/)
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH V7 2/3] arm64/mm: Hold memory hotplug lock while walking for kernel page table dump
From: Anshuman Khandual @ 2019-09-18 9:12 UTC (permalink / raw)
To: Balbir Singh, linux-mm, linux-kernel, linux-arm-kernel, akpm,
catalin.marinas, will
Cc: mark.rutland, mhocko, david, ira.weiny, steve.capper, mgorman,
steven.price, broonie, cai, ard.biesheuvel, cpandya, arunks,
dan.j.williams, Robin.Murphy, logang, valentin.schneider,
suzuki.poulose, osalvador
In-Reply-To: <66922798-9de7-a230-8548-1f205e79ea50@gmail.com>
On 09/15/2019 08:05 AM, Balbir Singh wrote:
>
>
> On 3/9/19 7:45 pm, Anshuman Khandual wrote:
>> The arm64 page table dump code can race with concurrent modification of the
>> kernel page tables. When a leaf entries are modified concurrently, the dump
>> code may log stale or inconsistent information for a VA range, but this is
>> otherwise not harmful.
>>
>> When intermediate levels of table are freed, the dump code will continue to
>> use memory which has been freed and potentially reallocated for another
>> purpose. In such cases, the dump code may dereference bogus addresses,
>> leading to a number of potential problems.
>>
>> Intermediate levels of table may by freed during memory hot-remove,
>> which will be enabled by a subsequent patch. To avoid racing with
>> this, take the memory hotplug lock when walking the kernel page table.
>>
>> Acked-by: David Hildenbrand <david@redhat.com>
>> Acked-by: Mark Rutland <mark.rutland@arm.com>
>> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
>> ---
>> arch/arm64/mm/ptdump_debugfs.c | 4 ++++
>> 1 file changed, 4 insertions(+)
>>
>> diff --git a/arch/arm64/mm/ptdump_debugfs.c b/arch/arm64/mm/ptdump_debugfs.c
>> index 064163f25592..b5eebc8c4924 100644
>> --- a/arch/arm64/mm/ptdump_debugfs.c
>> +++ b/arch/arm64/mm/ptdump_debugfs.c
>> @@ -1,5 +1,6 @@
>> // SPDX-License-Identifier: GPL-2.0
>> #include <linux/debugfs.h>
>> +#include <linux/memory_hotplug.h>
>> #include <linux/seq_file.h>
>>
>> #include <asm/ptdump.h>
>> @@ -7,7 +8,10 @@
>> static int ptdump_show(struct seq_file *m, void *v)
>> {
>> struct ptdump_info *info = m->private;
>> +
>> + get_online_mems();
>> ptdump_walk_pgd(m, info);
>> + put_online_mems();
>
> Looks sane, BTW, checking other arches they might have the same race.
The problem can be present on other architectures which can dump kernel page
table during memory hot-remove operation where it actually frees up page table
pages. If there is no freeing involved the race condition here could cause
inconsistent or garbage information capture for a given VA range. Same is true
even for concurrent vmalloc() operations as well. But removal of page tables
pages can make it worse. Freeing page table pages during hot-remove is a platform
decision, so would be adding these locks while walking kernel page table during
ptdump.
> Is there anything special about the arch?
AFAICS, no.
>
> Acked-by: Balbir Singh <bsingharora@gmail.com>
>
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* RE: [PATCH V6 2/2] mailbox: introduce ARM SMC based mailbox
From: Peng Fan @ 2019-09-18 9:09 UTC (permalink / raw)
To: Andre Przywara
Cc: mark.rutland@arm.com, devicetree@vger.kernel.org,
f.fainelli@gmail.com, jassisinghbrar@gmail.com,
linux-kernel@vger.kernel.org, robh+dt@kernel.org, dl-linux-imx,
sudeep.holla@arm.com, linux-arm-kernel@lists.infradead.org
In-Reply-To: <20190917183856.2342beed@donnerap.cambridge.arm.com>
Hi Andre,
> Subject: Re: [PATCH V6 2/2] mailbox: introduce ARM SMC based mailbox
>
> On Mon, 16 Sep 2019 09:44:41 +0000
> Peng Fan <peng.fan@nxp.com> wrote:
>
> Hi,
>
> looks quite good now, some smaller comments below.
> I think the only thing left is the "function ID passed by the client" topic.
>
> Have you tried using this interface using hvc, for instance in KVM or Xen? For
No. I do not have that implementation in hypervisor.
> instance to provide access to a clock for a passed-through platform device?
> Another use case that pops up from time to time is GPIO for guests. This
> sounds like a use case for using the register interface, also we could use the
> hvc return value.
>
> > From: Peng Fan <peng.fan@nxp.com>
> >
> > This mailbox driver implements a mailbox which signals transmitted
> > data via an ARM smc (secure monitor call) instruction. The mailbox
> > receiver is implemented in firmware and can synchronously return data
> > when it returns execution to the non-secure world again.
> > An asynchronous receive path is not implemented.
> > This allows the usage of a mailbox to trigger firmware actions on SoCs
> > which either don't have a separate management processor or on which
> > such a core is not available. A user of this mailbox could be the SCP
> > interface.
> >
> > Modified from Andre Przywara's v2 patch
> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore
> > .kernel.org%2Fpatchwork%2Fpatch%2F812999%2F&data=02%7C01%7
> Cpeng.fa
> >
> n%40nxp.com%7C58a1ed4078264d14958f08d73b95ed7e%7C686ea1d3bc2b
> 4c6fa92cd
> >
> 99c5c301635%7C0%7C1%7C637043387484474825&sdata=Cp1zlhlpQbg
> BsWu9ZDV
> > RKkXmd6kvUR%2BtPO7EPR7YLpA%3D&reserved=0
> >
> > Cc: Andre Przywara <andre.przywara@arm.com>
> > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > ---
> > drivers/mailbox/Kconfig | 7 ++
> > drivers/mailbox/Makefile | 2 +
> > drivers/mailbox/arm-smc-mailbox.c | 167
> > ++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 176 insertions(+)
> > create mode 100644 drivers/mailbox/arm-smc-mailbox.c
> >
> > diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig index
> > ab4eb750bbdd..7707ee26251a 100644
> > --- a/drivers/mailbox/Kconfig
> > +++ b/drivers/mailbox/Kconfig
> > @@ -16,6 +16,13 @@ config ARM_MHU
> > The controller has 3 mailbox channels, the last of which can be
> > used in Secure mode only.
> >
> > +config ARM_SMC_MBOX
> > + tristate "Generic ARM smc mailbox"
> > + depends on OF && HAVE_ARM_SMCCC
> > + help
> > + Generic mailbox driver which uses ARM smc calls to call into
> > + firmware for triggering mailboxes.
> > +
> > config IMX_MBOX
> > tristate "i.MX Mailbox"
> > depends on ARCH_MXC || COMPILE_TEST
> > diff --git a/drivers/mailbox/Makefile b/drivers/mailbox/Makefile index
> > c22fad6f696b..93918a84c91b 100644
> > --- a/drivers/mailbox/Makefile
> > +++ b/drivers/mailbox/Makefile
> > @@ -7,6 +7,8 @@ obj-$(CONFIG_MAILBOX_TEST) += mailbox-test.o
> >
> > obj-$(CONFIG_ARM_MHU) += arm_mhu.o
> >
> > +obj-$(CONFIG_ARM_SMC_MBOX) += arm-smc-mailbox.o
> > +
> > obj-$(CONFIG_IMX_MBOX) += imx-mailbox.o
> >
> > obj-$(CONFIG_ARMADA_37XX_RWTM_MBOX) +=
> armada-37xx-rwtm-mailbox.o
> > diff --git a/drivers/mailbox/arm-smc-mailbox.c
> > b/drivers/mailbox/arm-smc-mailbox.c
> > new file mode 100644
> > index 000000000000..c84aef39c8d9
> > --- /dev/null
> > +++ b/drivers/mailbox/arm-smc-mailbox.c
> > @@ -0,0 +1,167 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (C) 2016,2017 ARM Ltd.
> > + * Copyright 2019 NXP
> > + */
> > +
> > +#include <linux/arm-smccc.h>
> > +#include <linux/device.h>
> > +#include <linux/kernel.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/mailbox_controller.h> #include <linux/module.h>
> > +#include <linux/platform_device.h>
> > +
> > +struct arm_smc_chan_data {
> > + unsigned int function_id;
> > +};
> > +
> > +struct arm_smccc_mbox_cmd {
> > + unsigned int function_id;
> > + union {
> > + unsigned int args_smccc32[6];
> > + unsigned long args_smccc64[6];
>
> Shouldn't this be u32 and u64 here, as the data type has this explicit length in
> the SMCCC?
ok
>
> > + };
> > +};
>
> If this is the data structure that this mailbox controller uses, I would expect
> this to be documented somewhere, or even exported.
Export this structure in include/linux/mailbox/smc-mailbox.h?
>
> And again, I don't like the idea of having the function ID in here.
You mean function_id in arm_smccc_mbox_cmd is not good?
>
> > +
> > +typedef unsigned long (smc_mbox_fn)(unsigned int, unsigned long,
> > + unsigned long, unsigned long,
> > + unsigned long, unsigned long,
> > + unsigned long);
> > +static smc_mbox_fn *invoke_smc_mbox_fn;
> > +
> > +static int arm_smc_send_data(struct mbox_chan *link, void *data) {
> > + struct arm_smc_chan_data *chan_data = link->con_priv;
> > + struct arm_smccc_mbox_cmd *cmd = data;
> > + unsigned long ret;
> > + u32 function_id;
> > +
> > + function_id = chan_data->function_id;
> > + if (!function_id)
> > + function_id = cmd->function_id;
> > +
> > + if (function_id & BIT(30)) {
>
> if (ARM_SMCCC_IS_64(function_id)) {
ok
>
> > + ret = invoke_smc_mbox_fn(function_id, cmd->args_smccc64[0],
> > + cmd->args_smccc64[1],
> > + cmd->args_smccc64[2],
> > + cmd->args_smccc64[3],
> > + cmd->args_smccc64[4],
> > + cmd->args_smccc64[5]);
> > + } else {
> > + ret = invoke_smc_mbox_fn(function_id, cmd->args_smccc32[0],
> > + cmd->args_smccc32[1],
> > + cmd->args_smccc32[2],
> > + cmd->args_smccc32[3],
> > + cmd->args_smccc32[4],
> > + cmd->args_smccc32[5]);
> > + }
> > +
> > + mbox_chan_received_data(link, (void *)ret);
> > +
> > + return 0;
> > +}
> > +
> > +static unsigned long __invoke_fn_hvc(unsigned int function_id,
> > + unsigned long arg0, unsigned long arg1,
> > + unsigned long arg2, unsigned long arg3,
> > + unsigned long arg4, unsigned long arg5) {
> > + struct arm_smccc_res res;
> > +
> > + arm_smccc_hvc(function_id, arg0, arg1, arg2, arg3, arg4,
> > + arg5, 0, &res);
> > + return res.a0;
> > +}
> > +
> > +static unsigned long __invoke_fn_smc(unsigned int function_id,
> > + unsigned long arg0, unsigned long arg1,
> > + unsigned long arg2, unsigned long arg3,
> > + unsigned long arg4, unsigned long arg5) {
> > + struct arm_smccc_res res;
> > +
> > + arm_smccc_smc(function_id, arg0, arg1, arg2, arg3, arg4,
> > + arg5, 0, &res);
> > + return res.a0;
> > +}
> > +
> > +static const struct mbox_chan_ops arm_smc_mbox_chan_ops = {
> > + .send_data = arm_smc_send_data,
> > +};
> > +
> > +static int arm_smc_mbox_probe(struct platform_device *pdev) {
> > + struct device *dev = &pdev->dev;
> > + struct mbox_controller *mbox;
> > + struct arm_smc_chan_data *chan_data;
> > + int ret;
> > + u32 function_id = 0;
> > +
> > + if (of_device_is_compatible(dev->of_node, "arm,smc-mbox"))
> > + invoke_smc_mbox_fn = __invoke_fn_smc;
> > + else
> > + invoke_smc_mbox_fn = __invoke_fn_hvc;
> > +
> > + mbox = devm_kzalloc(dev, sizeof(*mbox), GFP_KERNEL);
> > + if (!mbox)
> > + return -ENOMEM;
> > +
> > + mbox->num_chans = 1;
> > + mbox->chans = devm_kzalloc(dev, sizeof(*mbox->chans), GFP_KERNEL);
> > + if (!mbox->chans)
> > + return -ENOMEM;
> > +
> > + chan_data = devm_kzalloc(dev, sizeof(*chan_data), GFP_KERNEL);
> > + if (!chan_data)
> > + return -ENOMEM;
> > +
> > + of_property_read_u32(dev->of_node, "arm,func-id", &function_id);
> > + chan_data->function_id = function_id;
> > +
> > + mbox->chans->con_priv = chan_data;
> > +
> > + mbox->txdone_poll = false;
> > + mbox->txdone_irq = false;
>
> Don't we need to provide something to confirm reception to the client? In our
> case we can set this as soon as the smc/hvc returns.
As smc/hvc returns, it means the transfer is over and receive is done.
Thanks,
Peng.
>
> Cheers,
> Andre.
>
> > + mbox->ops = &arm_smc_mbox_chan_ops;
> > + mbox->dev = dev;
> > +
> > + platform_set_drvdata(pdev, mbox);
> > +
> > + ret = devm_mbox_controller_register(dev, mbox);
> > + if (ret)
> > + return ret;
> > +
> > + dev_info(dev, "ARM SMC mailbox enabled.\n");
> > +
> > + return ret;
> > +}
> > +
> > +static int arm_smc_mbox_remove(struct platform_device *pdev) {
> > + struct mbox_controller *mbox = platform_get_drvdata(pdev);
> > +
> > + mbox_controller_unregister(mbox);
> > + return 0;
> > +}
> > +
> > +static const struct of_device_id arm_smc_mbox_of_match[] = {
> > + { .compatible = "arm,smc-mbox", },
> > + { .compatible = "arm,hvc-mbox", },
> > + {},
> > +};
> > +MODULE_DEVICE_TABLE(of, arm_smc_mbox_of_match);
> > +
> > +static struct platform_driver arm_smc_mbox_driver = {
> > + .driver = {
> > + .name = "arm-smc-mbox",
> > + .of_match_table = arm_smc_mbox_of_match,
> > + },
> > + .probe = arm_smc_mbox_probe,
> > + .remove = arm_smc_mbox_remove,
> > +};
> > +module_platform_driver(arm_smc_mbox_driver);
> > +
> > +MODULE_AUTHOR("Peng Fan <peng.fan@nxp.com>");
> > +MODULE_DESCRIPTION("Generic ARM smc mailbox driver");
> > +MODULE_LICENSE("GPL v2");
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH] [v2] arm64: fix unreachable code issue with cmpxchg
From: Will Deacon @ 2019-09-18 9:02 UTC (permalink / raw)
To: Nathan Chancellor
Cc: Mark Rutland, Arnd Bergmann, Peter Zijlstra (Intel),
Catalin Marinas, Nick Desaulniers, linux-kernel,
clang-built-linux, Andrew Murray, Thomas Gleixner,
linux-arm-kernel
In-Reply-To: <20190917203425.GA31423@archlinux-threadripper>
On Tue, Sep 17, 2019 at 01:34:25PM -0700, Nathan Chancellor wrote:
> On Tue, Sep 10, 2019 at 01:56:22PM +0200, Arnd Bergmann wrote:
> > On arm64 build with clang, sometimes the __cmpxchg_mb is not inlined
> > when CONFIG_OPTIMIZE_INLINING is set.
> > Clang then fails a compile-time assertion, because it cannot tell at
> > compile time what the size of the argument is:
> >
> > mm/memcontrol.o: In function `__cmpxchg_mb':
> > memcontrol.c:(.text+0x1a4c): undefined reference to `__compiletime_assert_175'
> > memcontrol.c:(.text+0x1a4c): relocation truncated to fit: R_AARCH64_CALL26 against undefined symbol `__compiletime_assert_175'
> >
> > Mark all of the cmpxchg() style functions as __always_inline to
> > ensure that the compiler can see the result.
> >
> > Acked-by: Nick Desaulniers <ndesaulniers@google.com>
> > Reported-by: Nathan Chancellor <natechancellor@gmail.com>
> > Link: https://github.com/ClangBuiltLinux/linux/issues/648
> > Reviewed-by: Nathan Chancellor <natechancellor@gmail.com>
> > Tested-by: Nathan Chancellor <natechancellor@gmail.com>
> > Reviewed-by: Andrew Murray <andrew.murray@arm.com>
> > Tested-by: Andrew Murray <andrew.murray@arm.com>
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > ---
> > v2: skip unneeded changes, as suggested by Andrew Murray
> > ---
> > arch/arm64/include/asm/cmpxchg.h | 6 +++---
> > 1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/arm64/include/asm/cmpxchg.h b/arch/arm64/include/asm/cmpxchg.h
> > index a1398f2f9994..f9bef42c1411 100644
> > --- a/arch/arm64/include/asm/cmpxchg.h
> > +++ b/arch/arm64/include/asm/cmpxchg.h
> > @@ -62,7 +62,7 @@ __XCHG_CASE( , , mb_, 64, dmb ish, nop, , a, l, "memory")
> > #undef __XCHG_CASE
> >
> > #define __XCHG_GEN(sfx) \
> > -static inline unsigned long __xchg##sfx(unsigned long x, \
> > +static __always_inline unsigned long __xchg##sfx(unsigned long x, \
> > volatile void *ptr, \
> > int size) \
> > { \
> > @@ -148,7 +148,7 @@ __CMPXCHG_DBL(_mb)
> > #undef __CMPXCHG_DBL
> >
> > #define __CMPXCHG_GEN(sfx) \
> > -static inline unsigned long __cmpxchg##sfx(volatile void *ptr, \
> > +static __always_inline unsigned long __cmpxchg##sfx(volatile void *ptr, \
> > unsigned long old, \
> > unsigned long new, \
> > int size) \
> > @@ -255,7 +255,7 @@ __CMPWAIT_CASE( , , 64);
> > #undef __CMPWAIT_CASE
> >
> > #define __CMPWAIT_GEN(sfx) \
> > -static inline void __cmpwait##sfx(volatile void *ptr, \
> > +static __always_inline void __cmpwait##sfx(volatile void *ptr, \
> > unsigned long val, \
> > int size) \
> > { \
> > --
> > 2.20.0
> >
>
> Looks like the arm64 pull request happened without this patch so clang
> all{mod,yes}config builds are broken. Did the maintainers have any
> further comments on it or could this make it in with the next one?
Fear not! I plan to send this with some other fixes we've got for -rc1.
I just to get my CI scripts going again (new machine), but that shouldn't
take long.
Will
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* RE: [PATCH V6 1/2] dt-bindings: mailbox: add binding doc for the ARM SMC/HVC mailbox
From: Peng Fan @ 2019-09-18 9:02 UTC (permalink / raw)
To: Andre Przywara
Cc: mark.rutland@arm.com, devicetree@vger.kernel.org,
f.fainelli@gmail.com, jassisinghbrar@gmail.com,
linux-kernel@vger.kernel.org, robh+dt@kernel.org, dl-linux-imx,
sudeep.holla@arm.com, linux-arm-kernel@lists.infradead.org
In-Reply-To: <20190917183115.3e40180f@donnerap.cambridge.arm.com>
Hi Andre,
> Subject: Re: [PATCH V6 1/2] dt-bindings: mailbox: add binding doc for the
> ARM SMC/HVC mailbox
>
> On Mon, 16 Sep 2019 09:44:37 +0000
> Peng Fan <peng.fan@nxp.com> wrote:
>
> Hi,
>
> > From: Peng Fan <peng.fan@nxp.com>
> >
> > The ARM SMC/HVC mailbox binding describes a firmware interface to
> > trigger actions in software layers running in the EL2 or EL3 exception levels.
> > The term "ARM" here relates to the SMC instruction as part of the ARM
> > instruction set, not as a standard endorsed by ARM Ltd.
> >
> > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > ---
> > .../devicetree/bindings/mailbox/arm-smc.yaml | 96
> ++++++++++++++++++++++
> > 1 file changed, 96 insertions(+)
> > create mode 100644
> > Documentation/devicetree/bindings/mailbox/arm-smc.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/mailbox/arm-smc.yaml
> > b/Documentation/devicetree/bindings/mailbox/arm-smc.yaml
> > new file mode 100644
> > index 000000000000..bf01bec035fc
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mailbox/arm-smc.yaml
> > @@ -0,0 +1,96 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) %YAML 1.2
> > +---
> > +$id:
> > +https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevi
> >
> +cetree.org%2Fschemas%2Fmailbox%2Farm-smc.yaml%23&data=02%7
> C01%7Cp
> >
> +eng.fan%40nxp.com%7Cff378bc3d622436c39ba08d73b94dfcc%7C686ea1d
> 3bc2b4c
> >
> +6fa92cd99c5c301635%7C0%7C1%7C637043382928045369&sdata=rnx
> KdDGjPPd
> > +8VBI5WmgnZ3jxIjL2hcRYzbljfFxDkA0%3D&reserved=0
> > +$schema:
> > +https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevi
> >
> +cetree.org%2Fmeta-schemas%2Fcore.yaml%23&data=02%7C01%7Cpe
> ng.fan%
> >
> +40nxp.com%7Cff378bc3d622436c39ba08d73b94dfcc%7C686ea1d3bc2b4c6
> fa92cd9
> >
> +9c5c301635%7C0%7C1%7C637043382928045369&sdata=R02nWzpp9
> %2BrDYG9tA
> > +ot4pdWb8tGGHet1MOjrD0dEjwA%3D&reserved=0
> > +
> > +title: ARM SMC Mailbox Interface
> > +
> > +maintainers:
> > + - Peng Fan <peng.fan@nxp.com>
> > +
> > +description: |
> > + This mailbox uses the ARM smc (secure monitor call) and hvc
> > +(hypervisor
>
> I think "or" instead of "and" is less confusing.
ok
>
> > + call) instruction to trigger a mailbox-connected activity in
> > + firmware, executing on the very same core as the caller. The value
> > + of r0/w0/x0 the firmware returns after the smc call is delivered as
> > + a received message to the mailbox framework, so synchronous
> > + communication can be established. The exact meaning of the action
> > + the mailbox triggers as well as the return value is defined by
> > + their users and is not subject to this binding.
> > +
> > + One use case of this mailbox is the SCMI interface, which uses
> > + shared
>
> One example use case of this mailbox ...
> (to make it more obvious that it's not restricted to this)
ok
>
> > + memory to transfer commands and parameters, and a mailbox to
> > + trigger a function call. This allows SoCs without a separate
> > + management processor (or when such a processor is not available or
> > + used) to use this standardized interface anyway.
> > +
> > + This binding describes no hardware, but establishes a firmware
> interface.
> > + Upon receiving an SMC using one of the described SMC function
> > + identifiers,
>
> ... the described SMC function identifier,
ok
>
> > + the firmware is expected to trigger some mailbox connected
> functionality.
> > + The communication follows the ARM SMC calling convention.
> > + Firmware expects an SMC function identifier in r0 or w0. The
> > + supported identifiers are passed from consumers,
>
> identifier
ok
>
> "passed from consumers": How? Where?
> But I want to repeat: We should not allow this. This is a binding for a mailbox
> controller driver, not a generic firmware backdoor.
As Jassi suggested the function identifier as an optional for mailbox driver.
The driver should support function id passed from consumers.
Currently there is no users for such case that passed from consumers,
so I have no idea how.
> We should be as strict as possible to avoid any security issues.
> The firmware certainly knows the function ID it implements. The firmware
> controls the DT. So it is straight-forward to put the ID into the DT. The
> firmware could even do this at boot time, dynamically, before passing on the
> DT to the non-secure world (bootloader or kernel).
>
> What would be the use case of this functionality?
>
> > or listed in the the arm,func-ids
>
> arm,func-id
ok
>
> > + properties as described below. The firmware can return one value in
>
> property
ok
>
> > + the first SMC result register, it is expected to be an error value,
> > + which shall be propagated to the mailbox client.
> > +
> > + Any core which supports the SMC or HVC instruction can be used, as
> > + long as a firmware component running in EL3 or EL2 is handling these
> calls.
> > +
> > +properties:
> > + compatible:
> > + oneOf:
> > + - description:
> > + For implementations using ARM SMC instruction.
> > + const: arm,smc-mbox
> > +
> > + - description:
> > + For implementations using ARM HVC instruction.
> > + const: arm,hvc-mbox
>
> I am not particularly happy with this, but well ...
>
> > +
> > + "#mbox-cells":
> > + const: 1
>
> Why is this "1"? What is this number used for? It used to be the channel ID,
> but since you are describing a single channel controller only, this should be 0
> now.
Mailbox bindings requires it at least 1, as replied to Jassi in the other mail.
>
> > +
> > + arm,func-id:
> > + description: |
> > + An 32-bit value specifying the function ID used by the mailbox.
>
> A single 32-bit value ...
>
> > + The function ID follow the ARM SMC calling convention standard
> [1].
>
> follows
>
> > + $ref: /schemas/types.yaml#/definitions/uint32
> > +
> > +required:
> > + - compatible
> > + - "#mbox-cells"
> > +
> > +examples:
> > + - |
> > + sram@93f000 {
> > + compatible = "mmio-sram";
> > + reg = <0x0 0x93f000 0x0 0x1000>;
> > + #address-cells = <1>;
> > + #size-cells = <1>;
> > + ranges = <0x0 0x93f000 0x1000>;
> > +
> > + cpu_scp_lpri: scp-shmem@0 {
> > + compatible = "arm,scmi-shmem";
> > + reg = <0x0 0x200>;
> > + };
> > + };
> > +
> > + smc_tx_mbox: tx_mbox {
> > + #mbox-cells = <1>;
>
> As mentioned above, should be 0.
>
> > + compatible = "arm,smc-mbox";
> > + /* optional */
>
> First: having "optional" in a specific example is not helpful, just confusing.
> Second: It is actually *not* optional in this case, as there is no other way of
> propagating the function ID. The SCMI driver as the mailbox client has
> certainly no clue about this.
I'll drop "/*optinal*/" since it is required in the example.
> I think I said this previously: Relying on the mailbox client to pass the function
> ID sounds broken, as this is a property of the mailbox controller driver. The
> mailbox client does not care about this mailbox communication detail, it just
> wants to trigger the mailbox.
>
> > + arm,func-id = <0xc20000fe>;
> > + };
> > +
> > + firmware {
> > + scmi {
> > + compatible = "arm,scmi";
> > + mboxes = <&smc_tx_mbox 0>;
>
> ... and here just <&smc_tx_mbox>; would suffice.
Mailbox requires mbox-cells at least 1, it must have one arg.
Otherwise of_mbox_index_xlate not work.
Thanks,
Peng.
>
> > + mbox-names = "tx";
> > + shmem = <&cpu_scp_lpri>;
> > + };
> > + };
> > +
> > +...
>
> Cheers,
> Andre.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* RE: [PATCH V6 1/2] dt-bindings: mailbox: add binding doc for the ARM SMC/HVC mailbox
From: Peng Fan @ 2019-09-18 8:53 UTC (permalink / raw)
To: Jassi Brar, Andre Przywara
Cc: mark.rutland@arm.com, devicetree@vger.kernel.org,
f.fainelli@gmail.com, linux-kernel@vger.kernel.org,
robh+dt@kernel.org, dl-linux-imx, sudeep.holla@arm.com,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <CABb+yY2CP1i9fZMoPua=-mLCUpYrcO28xF5UXDeRD2XTYe7mEg@mail.gmail.com>
Hi Jassi,
> Subject: Re: [PATCH V6 1/2] dt-bindings: mailbox: add binding doc for the
> ARM SMC/HVC mailbox
>
> On Tue, Sep 17, 2019 at 12:31 PM Andre Przywara
> <andre.przywara@arm.com> wrote:
> >
> > On Mon, 16 Sep 2019 09:44:37 +0000
> > Peng Fan <peng.fan@nxp.com> wrote:
> >
> > Hi,
> >
> > > From: Peng Fan <peng.fan@nxp.com>
> > >
> > > The ARM SMC/HVC mailbox binding describes a firmware interface to
> > > trigger actions in software layers running in the EL2 or EL3 exception
> levels.
> > > The term "ARM" here relates to the SMC instruction as part of the
> > > ARM instruction set, not as a standard endorsed by ARM Ltd.
> > >
> > > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > > ---
> > > .../devicetree/bindings/mailbox/arm-smc.yaml | 96
> ++++++++++++++++++++++
> > > 1 file changed, 96 insertions(+)
> > > create mode 100644
> > > Documentation/devicetree/bindings/mailbox/arm-smc.yaml
> > >
> > > diff --git a/Documentation/devicetree/bindings/mailbox/arm-smc.yaml
> > > b/Documentation/devicetree/bindings/mailbox/arm-smc.yaml
> > > new file mode 100644
> > > index 000000000000..bf01bec035fc
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/mailbox/arm-smc.yaml
> > > @@ -0,0 +1,96 @@
> > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) %YAML 1.2
> > > +---
> > > +$id:
> > > +https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fde
> > >
> +vicetree.org%2Fschemas%2Fmailbox%2Farm-smc.yaml%23&data=02%
> 7C01
> > >
> +%7Cpeng.fan%40nxp.com%7Cf8065d24dd474238baf008d73bf8dc7a%7C686
> ea1d3
> > >
> +bc2b4c6fa92cd99c5c301635%7C0%7C1%7C637043812342903260&sd
> ata=vC3
> > >
> +S8hvYDxDhNbIQoC44hpO5bw1yYZdBwu%2B%2Fp8mV0hI%3D&reserv
> ed=0
> > > +$schema:
> > > +https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fde
> > >
> +vicetree.org%2Fmeta-schemas%2Fcore.yaml%23&data=02%7C01%7C
> peng.
> > >
> +fan%40nxp.com%7Cf8065d24dd474238baf008d73bf8dc7a%7C686ea1d3bc2
> b4c6f
> > >
> +a92cd99c5c301635%7C0%7C1%7C637043812342903260&sdata=IDHd
> vf1Mgw1
> > > +BR%2Bo4XJ%2BjQS%2Bx1pSBzADnW44B2hZLzKw%3D&reserved=0
> > > +
> > > +title: ARM SMC Mailbox Interface
> > > +
> > > +maintainers:
> > > + - Peng Fan <peng.fan@nxp.com>
> > > +
> > > +description: |
> > > + This mailbox uses the ARM smc (secure monitor call) and hvc
> > > +(hypervisor
> >
> > I think "or" instead of "and" is less confusing.
> >
> > > + call) instruction to trigger a mailbox-connected activity in
> > > + firmware, executing on the very same core as the caller. The
> > > + value of r0/w0/x0 the firmware returns after the smc call is
> > > + delivered as a received message to the mailbox framework, so
> > > + synchronous communication can be established. The exact meaning
> > > + of the action the mailbox triggers as well as the return value is
> > > + defined by their users and is not subject to this binding.
> > > +
> > > + One use case of this mailbox is the SCMI interface, which uses
> > > + shared
> >
> > One example use case of this mailbox ...
> > (to make it more obvious that it's not restricted to this)
> >
> > > + memory to transfer commands and parameters, and a mailbox to
> > > + trigger a function call. This allows SoCs without a separate
> > > + management processor (or when such a processor is not available
> > > + or used) to use this standardized interface anyway.
> > > +
> > > + This binding describes no hardware, but establishes a firmware
> interface.
> > > + Upon receiving an SMC using one of the described SMC function
> > > + identifiers,
> >
> > ... the described SMC function
> > identifier,
> >
> > > + the firmware is expected to trigger some mailbox connected
> functionality.
> > > + The communication follows the ARM SMC calling convention.
> > > + Firmware expects an SMC function identifier in r0 or w0. The
> > > + supported identifiers are passed from consumers,
> >
> > identifier
> >
> > "passed from consumers": How? Where?
> > But I want to repeat: We should not allow this.
> > This is a binding for a mailbox controller driver, not a generic firmware
> backdoor.
> >
> Exactly. The mailbox controller here is the SMC/HVC instruction, which
> needs 9 arguments to work. The fact that the fist argument is always going to
> be same on a platform is just the way we use this instruction.
>
> > We should be as strict as possible to avoid any security issues.
> >
> Any example of such a security issue?
>
> > The firmware certainly knows the function ID it implements. The firmware
> controls the DT. So it is straight-forward to put the ID into the DT. The
> firmware could even do this at boot time, dynamically, before passing on the
> DT to the non-secure world (bootloader or kernel).
> >
> > What would be the use case of this functionality?
> >
> At least for flexibility and consistency.
>
> > > or listed in the the arm,func-ids
> >
> > arm,func-id
> >
> > > + properties as described below. The firmware can return one value
> > > + in
> >
> > property
> >
> > > + the first SMC result register, it is expected to be an error
> > > + value, which shall be propagated to the mailbox client.
> > > +
> > > + Any core which supports the SMC or HVC instruction can be used,
> > > + as long as a firmware component running in EL3 or EL2 is handling
> these calls.
> > > +
> > > +properties:
> > > + compatible:
> > > + oneOf:
> > > + - description:
> > > + For implementations using ARM SMC instruction.
> > > + const: arm,smc-mbox
> > > +
> > > + - description:
> > > + For implementations using ARM HVC instruction.
> > > + const: arm,hvc-mbox
> >
> > I am not particularly happy with this, but well ...
> >
> > > +
> > > + "#mbox-cells":
> > > + const: 1
> >
> > Why is this "1"? What is this number used for? It used to be the channel ID,
> but since you are describing a single channel controller only, this should be 0
> now.
> >
> Yes. I overlooked it and actually queued the patch for pull request.
In Documentation/devicetree/bindings/mailbox/mailbox.txt
#mbox-cells: Must be at least 1.
So I use 1 here, 0 not work. Because of_mbox_index_xlate expect at least 1 here.
So I need modify Documentation/devicetree/bindings/mailbox/mailbox.txt
and add xlate for smc mailbox?
Thanks,
Peng.
> But I think the bindings should not carry a 'fix' patch later. Also I realise this
> revision of binding hasn't been reviewed by Rob. Maybe I should drop the
> patch for now.
>
> > > +
> > > + arm,func-id:
> > > + description: |
> > > + An 32-bit value specifying the function ID used by the mailbox.
> >
> > A single 32-bit value ...
> >
> > > + The function ID follow the ARM SMC calling convention standard
> [1].
> >
> > follows
> >
> > > + $ref: /schemas/types.yaml#/definitions/uint32
> > > +
> > > +required:
> > > + - compatible
> > > + - "#mbox-cells"
> > > +
> > > +examples:
> > > + - |
> > > + sram@93f000 {
> > > + compatible = "mmio-sram";
> > > + reg = <0x0 0x93f000 0x0 0x1000>;
> > > + #address-cells = <1>;
> > > + #size-cells = <1>;
> > > + ranges = <0x0 0x93f000 0x1000>;
> > > +
> > > + cpu_scp_lpri: scp-shmem@0 {
> > > + compatible = "arm,scmi-shmem";
> > > + reg = <0x0 0x200>;
> > > + };
> > > + };
> > > +
> > > + smc_tx_mbox: tx_mbox {
> > > + #mbox-cells = <1>;
> >
> > As mentioned above, should be 0.
> >
> > > + compatible = "arm,smc-mbox";
> > > + /* optional */
> >
> > First: having "optional" in a specific example is not helpful, just confusing.
> > Second: It is actually *not* optional in this case, as there is no other way of
> propagating the function ID. The SCMI driver as the mailbox client has
> certainly no clue about this.
> > I think I said this previously: Relying on the mailbox client to pass the
> function ID sounds broken, as this is a property of the mailbox controller driver.
> The mailbox client does not care about this mailbox communication detail, it
> just wants to trigger the mailbox.
> >
> Again, the mailbox controller here is the SMC/HVC _instruction_, which
> doesn't care what value the first argument carry.
>
> Cheers!
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v2 1/7] counter: Simplify the count_read and count_write callbacks
From: Benjamin Gaignard @ 2019-09-18 8:48 UTC (permalink / raw)
To: William Breathitt Gray
Cc: Jonathan Cameron, Alexandre Torgue, linux-iio, Patrick Havelange,
Linux Kernel Mailing List, Maxime Coquelin, Fabrice Gasnier,
linux-stm32, Linux ARM
In-Reply-To: <f7969048e5db977cc6cc9daa8d32b170cf9f4c17.1568792697.git.vilhelm.gray@gmail.com>
Le mer. 18 sept. 2019 à 09:53, William Breathitt Gray
<vilhelm.gray@gmail.com> a écrit :
>
> The count_read and count_write callbacks are simplified to pass val as
> unsigned long rather than as an opaque data structure. The opaque
> counter_count_read_value and counter_count_write_value structures,
> counter_count_value_type enum, and relevant counter_count_read_value_set
> and counter_count_write_value_get functions, are removed as they are no
> longer used.
>
> Signed-off-by: William Breathitt Gray <vilhelm.gray@gmail.com>
Hi William,
This first patch break the compilation because you remove some
structure needed for the drivers.
You should merge all the serie in 1 patch to avoid that.
Benjamin
> ---
> drivers/counter/counter.c | 66 +++++----------------------------------
> include/linux/counter.h | 43 +++----------------------
> 2 files changed, 12 insertions(+), 97 deletions(-)
>
> diff --git a/drivers/counter/counter.c b/drivers/counter/counter.c
> index 106bc7180cd8..1d08f1437b1b 100644
> --- a/drivers/counter/counter.c
> +++ b/drivers/counter/counter.c
> @@ -246,60 +246,6 @@ void counter_signal_read_value_set(struct counter_signal_read_value *const val,
> }
> EXPORT_SYMBOL_GPL(counter_signal_read_value_set);
>
> -/**
> - * counter_count_read_value_set - set counter_count_read_value data
> - * @val: counter_count_read_value structure to set
> - * @type: property Count data represents
> - * @data: Count data
> - *
> - * This function sets an opaque counter_count_read_value structure with the
> - * provided Count data.
> - */
> -void counter_count_read_value_set(struct counter_count_read_value *const val,
> - const enum counter_count_value_type type,
> - void *const data)
> -{
> - switch (type) {
> - case COUNTER_COUNT_POSITION:
> - val->len = sprintf(val->buf, "%lu\n", *(unsigned long *)data);
> - break;
> - default:
> - val->len = 0;
> - }
> -}
> -EXPORT_SYMBOL_GPL(counter_count_read_value_set);
> -
> -/**
> - * counter_count_write_value_get - get counter_count_write_value data
> - * @data: Count data
> - * @type: property Count data represents
> - * @val: counter_count_write_value structure containing data
> - *
> - * This function extracts Count data from the provided opaque
> - * counter_count_write_value structure and stores it at the address provided by
> - * @data.
> - *
> - * RETURNS:
> - * 0 on success, negative error number on failure.
> - */
> -int counter_count_write_value_get(void *const data,
> - const enum counter_count_value_type type,
> - const struct counter_count_write_value *const val)
> -{
> - int err;
> -
> - switch (type) {
> - case COUNTER_COUNT_POSITION:
> - err = kstrtoul(val->buf, 0, data);
> - if (err)
> - return err;
> - break;
> - }
> -
> - return 0;
> -}
> -EXPORT_SYMBOL_GPL(counter_count_write_value_get);
> -
> struct counter_attr_parm {
> struct counter_device_attr_group *group;
> const char *prefix;
> @@ -788,13 +734,13 @@ static ssize_t counter_count_show(struct device *dev,
> const struct counter_count_unit *const component = devattr->component;
> struct counter_count *const count = component->count;
> int err;
> - struct counter_count_read_value val = { .buf = buf };
> + unsigned long val;
>
> err = counter->ops->count_read(counter, count, &val);
> if (err)
> return err;
>
> - return val.len;
> + return sprintf(buf, "%lu\n", val);
> }
>
> static ssize_t counter_count_store(struct device *dev,
> @@ -806,9 +752,13 @@ static ssize_t counter_count_store(struct device *dev,
> const struct counter_count_unit *const component = devattr->component;
> struct counter_count *const count = component->count;
> int err;
> - struct counter_count_write_value val = { .buf = buf };
> + unsigned long val;
> +
> + err = kstrtoul(buf, 0, &val);
> + if (err)
> + return err;
>
> - err = counter->ops->count_write(counter, count, &val);
> + err = counter->ops->count_write(counter, count, val);
> if (err)
> return err;
>
> diff --git a/include/linux/counter.h b/include/linux/counter.h
> index a061cdcdef7c..7e40796598a6 100644
> --- a/include/linux/counter.h
> +++ b/include/linux/counter.h
> @@ -300,24 +300,6 @@ struct counter_signal_read_value {
> size_t len;
> };
>
> -/**
> - * struct counter_count_read_value - Opaque Count read value
> - * @buf: string representation of Count read value
> - * @len: length of string in @buf
> - */
> -struct counter_count_read_value {
> - char *buf;
> - size_t len;
> -};
> -
> -/**
> - * struct counter_count_write_value - Opaque Count write value
> - * @buf: string representation of Count write value
> - */
> -struct counter_count_write_value {
> - const char *buf;
> -};
> -
> /**
> * struct counter_ops - Callbacks from driver
> * @signal_read: optional read callback for Signal attribute. The read
> @@ -328,15 +310,10 @@ struct counter_count_write_value {
> * signal_read callback.
> * @count_read: optional read callback for Count attribute. The read
> * value of the respective Count should be passed back via
> - * the val parameter. val points to an opaque type which
> - * should be set only by calling the
> - * counter_count_read_value_set function from within the
> - * count_read callback.
> + * the val parameter.
> * @count_write: optional write callback for Count attribute. The write
> * value for the respective Count is passed in via the val
> - * parameter. val points to an opaque type which should be
> - * accessed only by calling the
> - * counter_count_write_value_get function.
> + * parameter.
> * @function_get: function to get the current count function mode. Returns
> * 0 on success and negative error code on error. The index
> * of the respective Count's returned function mode should
> @@ -357,11 +334,9 @@ struct counter_ops {
> struct counter_signal *signal,
> struct counter_signal_read_value *val);
> int (*count_read)(struct counter_device *counter,
> - struct counter_count *count,
> - struct counter_count_read_value *val);
> + struct counter_count *count, unsigned long *val);
> int (*count_write)(struct counter_device *counter,
> - struct counter_count *count,
> - struct counter_count_write_value *val);
> + struct counter_count *count, unsigned long val);
> int (*function_get)(struct counter_device *counter,
> struct counter_count *count, size_t *function);
> int (*function_set)(struct counter_device *counter,
> @@ -486,19 +461,9 @@ enum counter_signal_value_type {
> COUNTER_SIGNAL_LEVEL = 0
> };
>
> -enum counter_count_value_type {
> - COUNTER_COUNT_POSITION = 0,
> -};
> -
> void counter_signal_read_value_set(struct counter_signal_read_value *const val,
> const enum counter_signal_value_type type,
> void *const data);
> -void counter_count_read_value_set(struct counter_count_read_value *const val,
> - const enum counter_count_value_type type,
> - void *const data);
> -int counter_count_write_value_get(void *const data,
> - const enum counter_count_value_type type,
> - const struct counter_count_write_value *const val);
>
> int counter_register(struct counter_device *const counter);
> void counter_unregister(struct counter_device *const counter);
> --
> 2.23.0
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH v2] of: restore old handling of cells_name=NULL in of_*_phandle_with_args()
From: Uwe Kleine-König @ 2019-09-18 8:47 UTC (permalink / raw)
To: Peter Rosin, Rob Herring, Frank Rowand
Cc: devicetree@vger.kernel.org, Geert Uytterhoeven, Robin Murphy,
Joerg Roedel, linux-kernel@vger.kernel.org,
linux-mediatek@lists.infradead.org, kernel@pengutronix.de,
Matthias Brugger, Geert Uytterhoeven, Will Deacon,
linux-arm-kernel@lists.infradead.org, Marek Szyprowski
In-Reply-To: <b00ca30f-2c06-7722-96b2-123d15751cb6@axentia.se>
Before commit e42ee61017f5 ("of: Let of_for_each_phandle fallback to
non-negative cell_count") the iterator functions calling
of_for_each_phandle assumed a cell count of 0 if cells_name was NULL.
This corner case was missed when implementing the fallback logic in
e42ee61017f5 and resulted in an endless loop.
Restore the old behaviour of of_count_phandle_with_args() and
of_parse_phandle_with_args() and add a check to
of_phandle_iterator_init() to prevent a similar failure as a safety
precaution. of_parse_phandle_with_args_map() doesn't need a similar fix
as cells_name isn't NULL there.
Affected drivers are:
- drivers/base/power/domain.c
- drivers/base/power/domain.c
- drivers/clk/ti/clk-dra7-atl.c
- drivers/hwmon/ibmpowernv.c
- drivers/i2c/muxes/i2c-demux-pinctrl.c
- drivers/iommu/mtk_iommu.c
- drivers/net/ethernet/freescale/fman/mac.c
- drivers/opp/of.c
- drivers/perf/arm_dsu_pmu.c
- drivers/regulator/of_regulator.c
- drivers/remoteproc/imx_rproc.c
- drivers/soc/rockchip/pm_domains.c
- sound/soc/fsl/imx-audmix.c
- sound/soc/fsl/imx-audmix.c
- sound/soc/meson/axg-card.c
- sound/soc/samsung/tm2_wm5110.c
- sound/soc/samsung/tm2_wm5110.c
Thanks to Geert Uytterhoeven for reporting the issue, Peter Rosin for
helping pinpoint the actual problem and the testers for confirming this
fix.
Fixes: e42ee61017f5 ("of: Let of_for_each_phandle fallback to non-negative cell_count")
Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
On Wed, Sep 18, 2019 at 08:01:05AM +0000, Peter Rosin wrote:
> On 2019-09-18 08:38, Uwe Kleine-König wrote:
> > EXPORT_SYMBOL(of_parse_phandle_with_args);
> >
> > @@ -1765,6 +1779,18 @@ int of_count_phandle_with_args(const struct device_node *np, const char *list_na
> > struct of_phandle_iterator it;
> > int rc, cur_index = 0;
> >
> > + /* If cells_name is NULL we assume a cell count of 0 */
> > + if (cells_name == NULL) {
>
> A couple of nits.
>
> I don't know if there are other considerations, but in the previous two
> hunks you use !cells_name instead of comparing explicitly with NULL.
> Personally, I find the shorter form more readable, and in the name of
> consistency bla bla...
Ack, changed to !cells_name here, too.
>
> Also, the comment explaining this NULL-check didn't really make sense
> to me until I realized that knowing the cell count to be zero makes
> counting trivial. Something along those lines should perhaps be in the
> comment?
You're right, I extended the comment a bit.
> But as I said, these are nits. Feel free to ignore.
I considered resending already anyhow as I fatfingerd my email address.
this is fixed now, too. Additionally I fixed a typo in one of the
comments.
Thanks for your feedback.
Best regards
Uwe
drivers/of/base.c | 35 +++++++++++++++++++++++++++++++++--
1 file changed, 33 insertions(+), 2 deletions(-)
diff --git a/drivers/of/base.c b/drivers/of/base.c
index 2f25d2dfecfa..1d667eb730e1 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -1286,6 +1286,13 @@ int of_phandle_iterator_init(struct of_phandle_iterator *it,
memset(it, 0, sizeof(*it));
+ /*
+ * one of cell_count or cells_name must be provided to determine the
+ * argument length.
+ */
+ if (cell_count < 0 && !cells_name)
+ return -EINVAL;
+
list = of_get_property(np, list_name, &size);
if (!list)
return -ENOENT;
@@ -1512,10 +1519,17 @@ int of_parse_phandle_with_args(const struct device_node *np, const char *list_na
const char *cells_name, int index,
struct of_phandle_args *out_args)
{
+ int cell_count = -1;
+
if (index < 0)
return -EINVAL;
- return __of_parse_phandle_with_args(np, list_name, cells_name, -1,
- index, out_args);
+
+ /* If cells_name is NULL we assume a cell count of 0 */
+ if (!cells_name)
+ cell_count = 0;
+
+ return __of_parse_phandle_with_args(np, list_name, cells_name,
+ cell_count, index, out_args);
}
EXPORT_SYMBOL(of_parse_phandle_with_args);
@@ -1765,6 +1779,23 @@ int of_count_phandle_with_args(const struct device_node *np, const char *list_na
struct of_phandle_iterator it;
int rc, cur_index = 0;
+ /*
+ * If cells_name is NULL we assume a cell count of 0. This makes
+ * counting the phandles trivial as each 32bit word in the list is a
+ * phandle and no arguments are to consider. So we don't iterate through
+ * the list but just use the length to determine the phandle count.
+ */
+ if (!cells_name) {
+ const __be32 *list;
+ int size;
+
+ list = of_get_property(np, list_name, &size);
+ if (!list)
+ return -ENOENT;
+
+ return size / sizeof(*list);
+ }
+
rc = of_phandle_iterator_init(&it, np, list_name, cells_name, -1);
if (rc)
return rc;
--
2.23.0
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [PATCH v6 2/4] drm: dw-hdmi-i2s: Use fixed id for codec device
From: Jerome Brunet @ 2019-09-18 8:43 UTC (permalink / raw)
To: Cheng-Yi Chiang
Cc: alsa-devel, Heiko Stuebner, Neil Armstrong, David Airlie,
dri-devel, linux-kernel, Hans Verkuil, Andrzej Hajda,
Laurent Pinchart, Takashi Iwai, linux-rockchip, dgreid, tzungbi,
Jonas Karlman, Liam Girdwood, Russell King, Mark Brown,
Jaroslav Kysela, linux-arm-kernel, Jernej Skrabec, dianders,
Daniel Vetter
In-Reply-To: <20190918082500.209281-3-cychiang@chromium.org>
On Wed 18 Sep 2019 at 10:24, Cheng-Yi Chiang <cychiang@chromium.org> wrote:
> The problem of using auto ID is that the device name will be like
> hdmi-audio-codec.<id number>.auto.
>
> The number might be changed when there are other platform devices being
> created before hdmi-audio-codec device.
> Use a fixed name so machine driver can set codec name on the DAI link.
>
> Using the fixed name should be fine because there will only be one
> hdmi-audio-codec device.
While this is true all platforms we know of (I suppose), It might not be
the case later on. I wonder if making such assumption is really
desirable in a code which is used by quite a few different platforms.
Instead of trying to predict what the device name will be, can't you just
query it in your machine driver ? Using a device tree phandle maybe ?
It is quite usual to set the dai links this way, "simple-card" is a good
example of this.
>
> Fix the codec name in rockchip rk3288_hdmi_analog machine driver.
>
> Signed-off-by: Cheng-Yi Chiang <cychiang@chromium.org>
> ---
> drivers/gpu/drm/bridge/synopsys/dw-hdmi-i2s-audio.c | 2 +-
> sound/soc/rockchip/rk3288_hdmi_analog.c | 3 ++-
> 2 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-i2s-audio.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-i2s-audio.c
> index d7e65c869415..86bd482b9f94 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-i2s-audio.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-i2s-audio.c
> @@ -193,7 +193,7 @@ static int snd_dw_hdmi_probe(struct platform_device *pdev)
>
> memset(&pdevinfo, 0, sizeof(pdevinfo));
> pdevinfo.parent = pdev->dev.parent;
> - pdevinfo.id = PLATFORM_DEVID_AUTO;
> + pdevinfo.id = PLATFORM_DEVID_NONE;
> pdevinfo.name = HDMI_CODEC_DRV_NAME;
> pdevinfo.data = &pdata;
> pdevinfo.size_data = sizeof(pdata);
> diff --git a/sound/soc/rockchip/rk3288_hdmi_analog.c b/sound/soc/rockchip/rk3288_hdmi_analog.c
> index 767700c34ee2..8286025a8747 100644
> --- a/sound/soc/rockchip/rk3288_hdmi_analog.c
> +++ b/sound/soc/rockchip/rk3288_hdmi_analog.c
> @@ -15,6 +15,7 @@
> #include <linux/gpio.h>
> #include <linux/of_gpio.h>
> #include <sound/core.h>
> +#include <sound/hdmi-codec.h>
> #include <sound/jack.h>
> #include <sound/pcm.h>
> #include <sound/pcm_params.h>
> @@ -142,7 +143,7 @@ static const struct snd_soc_ops rk_ops = {
> SND_SOC_DAILINK_DEFS(audio,
> DAILINK_COMP_ARRAY(COMP_EMPTY()),
> DAILINK_COMP_ARRAY(COMP_CODEC(NULL, NULL),
> - COMP_CODEC("hdmi-audio-codec.2.auto", "i2s-hifi")),
> + COMP_CODEC(HDMI_CODEC_DRV_NAME, "i2s-hifi")),
> DAILINK_COMP_ARRAY(COMP_EMPTY()));
>
> static struct snd_soc_dai_link rk_dailink = {
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v6 0/3] coresight: etm4x: save/restore ETMv4 context across CPU low power states
From: Andrew Murray @ 2019-09-18 8:36 UTC (permalink / raw)
To: Mathieu Poirier
Cc: Mark Rutland, devicetree, Al Grant, Suzuki K Poulose,
Alexander Shishkin, Coresight ML, Leo Yan, Rob Herring,
Sudeep Holla, linux-arm-kernel, Mike Leach
In-Reply-To: <CANLsYkzhB3OBpLTAR54p771OSLxj+NYF8_kE=aMp7SkdNzaxwA@mail.gmail.com>
On Tue, Sep 17, 2019 at 01:37:10PM -0600, Mathieu Poirier wrote:
> Hi Andrew,
>
> On Fri, 13 Sep 2019 at 05:53, Andrew Murray <andrew.murray@arm.com> wrote:
>
> [...]
>
> >
> > Andrew Murray (3):
> > coresight: etm4x: save/restore state across CPU low power states
> > dt-bindings: arm: coresight: Add support for
> > coresight-loses-context-with-cpu
>
> I have picked-up patches 1 and 2. As per the conversation we had in
> Cambridge where we kept finding ways to break things when dealing with
> an external agent, I have not applied the 3 patch.
Thanks for the picking these up. No worries about patch 3.
Thanks,
Andrew Murray
>
> Thanks,
> Mathieu
>
> > coresight: etm4x: save/restore state for external agents
> >
> > .../devicetree/bindings/arm/coresight.txt | 9 +
> > drivers/hwtracing/coresight/coresight-etm4x.c | 351 +++++++++++++++++-
> > drivers/hwtracing/coresight/coresight-etm4x.h | 64 ++++
> > drivers/hwtracing/coresight/coresight.c | 8 +-
> > include/linux/coresight.h | 13 +
> > 5 files changed, 443 insertions(+), 2 deletions(-)
> >
> > --
> > 2.21.0
> >
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [RFC PATCH V4 3/4] dt-bindings: mt8183: Add sensor interface dt-bindings
From: Sakari Ailus @ 2019-09-18 8:29 UTC (permalink / raw)
To: Louis Kuo
Cc: devicetree, Sean.Cheng, laurent.pinchart+renesas, Rynn.Wu,
christie.yu, srv_heupstream, Jerry-ch.Chen, tfiga, keiichiw,
jungo.lin, sj.huang, yuzhao, hans.verkuil, zwisler, frederic.chen,
matthias.bgg, linux-mediatek, mchehab, linux-arm-kernel,
linux-media
In-Reply-To: <20190915065004.20257-4-louis.kuo@mediatek.com>
Hi Louis,
On Sun, Sep 15, 2019 at 02:50:03PM +0800, Louis Kuo wrote:
> This patch adds the DT binding documentation for the sensor interface
> module in Mediatek SoCs.
>
> Signed-off-by: Louis Kuo <louis.kuo@mediatek.com>
> ---
> .../bindings/media/mediatek-seninf.txt | 30 +++++++++++++++++++
> 1 file changed, 30 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/media/mediatek-seninf.txt
>
> diff --git a/Documentation/devicetree/bindings/media/mediatek-seninf.txt b/Documentation/devicetree/bindings/media/mediatek-seninf.txt
> new file mode 100644
> index 000000000000..bf2eb801cb47
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/mediatek-seninf.txt
> @@ -0,0 +1,30 @@
> +* Mediatek seninf MIPI-CSI2 host driver
> +
> +Seninf MIPI-CSI2 host driver is a HW camera interface controller. It support a widely adopted,
> +simple, high-speed protocol primarily intended for point-to-point image and video
> +transmission between cameras and host devices.
Please wrap so it fits to 80 chars per line.
You'll also need to document the graph nodes needed. Which ports correspond
to which interfaces on the device, for instance?
Please document which endpoint properties are required and whether there
are any optional ones.
> +
> +Required properties:
> + - compatible: "mediatek,mt8183-seninf"
> + - reg: Must contain an entry for each entry in reg-names.
> + - reg-names: Must include the following entries:
> + "base_reg": seninf registers base
> + "rx_reg": Rx analog registers base
> + - interrupts: interrupt number to the cpu.
> + - clocks : clock name from clock manager
> + - clock-names: must be CLK_CAM_SENINF and CLK_TOP_MUX_SENINF.
> + It is the clocks of seninf
> +
> +Example:
> + seninf: seninf@1a040000 {
> + compatible = "mediatek,mt8183-seninf";
> + reg = <0 0x1a040000 0 0x8000>,
> + <0 0x11C80000 0 0x6000>;
> + reg-names = "base_reg", "rx_reg";
> + interrupts = <GIC_SPI 251 IRQ_TYPE_LEVEL_LOW>;
> + power-domains = <&scpsys MT8183_POWER_DOMAIN_CAM>;
> + clocks = <&camsys CLK_CAM_SENINF>,
> + <&topckgen CLK_TOP_MUX_SENINF>;
> + clock-names = "CLK_CAM_SENINF", "CLK_TOP_MUX_SENINF";
> + }
> +
--
Regards,
Sakari Ailus
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [RFC PATCH v3 3/6] timekeeping: Expose API allowing retrival of current clocksource and counter value
From: Paolo Bonzini @ 2019-09-18 8:29 UTC (permalink / raw)
To: Jianyong Wu, netdev, yangbo.lu, john.stultz, tglx,
sean.j.christopherson, maz, richardcochran, Mark.Rutland,
Will.Deacon, suzuki.poulose
Cc: justin.he, kvm, Steve.Capper, linux-kernel, Kaly.Xin, nd,
linux-arm-kernel
In-Reply-To: <20190918080716.64242-4-jianyong.wu@arm.com>
On 18/09/19 10:07, Jianyong Wu wrote:
> From Marc Zyngier <maz@kernel.org>
> A number of PTP drivers (such as ptp-kvm) are assuming what the
> current clock source is, which could lead to interesting effects on
> systems where the clocksource can change depending on external events.
>
> For this purpose, add a new API that retrives both the current
> monotonic clock as well as its counter value.
>
> From Jianyong Wu: export this API then modules can use it.
See review of patch 4. ktime_get_snapshot is even better for your
needs, if I'm not mistaken.
Paolo
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> Signed-off-by: Jianyong Wu <jianyong.wu@arm.com>
> ---
> include/linux/timekeeping.h | 3 +++
> kernel/time/timekeeping.c | 13 +++++++++++++
> 2 files changed, 16 insertions(+)
>
> diff --git a/include/linux/timekeeping.h b/include/linux/timekeeping.h
> index a8ab0f143ac4..a5389adaa8bc 100644
> --- a/include/linux/timekeeping.h
> +++ b/include/linux/timekeeping.h
> @@ -247,6 +247,9 @@ extern int get_device_system_crosststamp(
> struct system_time_snapshot *history,
> struct system_device_crosststamp *xtstamp);
>
> +/* Obtain current monotonic clock and its counter value */
> +extern void get_current_counterval(struct system_counterval_t *sc);
> +
> /*
> * Simultaneously snapshot realtime and monotonic raw clocks
> */
> diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
> index 44b726bab4bd..07a0969625b1 100644
> --- a/kernel/time/timekeeping.c
> +++ b/kernel/time/timekeeping.c
> @@ -1098,6 +1098,19 @@ static bool cycle_between(u64 before, u64 test, u64 after)
> return false;
> }
>
> +/**
> + * get_current_counterval - Snapshot the current clocksource and counter value
> + * @sc: Pointer to a struct containing the current clocksource and its value
> + */
> +void get_current_counterval(struct system_counterval_t *sc)
> +{
> + struct timekeeper *tk = &tk_core.timekeeper;
> +
> + sc->cs = READ_ONCE(tk->tkr_mono.clock);
> + sc->cycles = sc->cs->read(sc->cs);
> +}
> +EXPORT_SYMBOL_GPL(get_current_counterval);
> +
> /**
> * get_device_system_crosststamp - Synchronously capture system/device timestamp
> * @get_time_fn: Callback to get simultaneous device time and
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH v6 4/4] ASoC: rockchip_max98090: Add HDMI jack support
From: Cheng-Yi Chiang @ 2019-09-18 8:25 UTC (permalink / raw)
To: linux-kernel
Cc: alsa-devel, Heiko Stuebner, Neil Armstrong, David Airlie,
dri-devel, Liam Girdwood, Hans Verkuil, Andrzej Hajda,
Laurent Pinchart, Jerome Brunet, Takashi Iwai, linux-rockchip,
dgreid, Cheng-Yi Chiang, tzungbi, Jonas Karlman, Russell King,
Mark Brown, Jaroslav Kysela, linux-arm-kernel, Jernej Skrabec,
dianders, Daniel Vetter
In-Reply-To: <20190918082500.209281-1-cychiang@chromium.org>
In machine driver, create a jack and let hdmi-codec report jack status.
Signed-off-by: Cheng-Yi Chiang <cychiang@chromium.org>
---
sound/soc/rockchip/Kconfig | 3 ++-
sound/soc/rockchip/rockchip_max98090.c | 20 ++++++++++++++++++++
2 files changed, 22 insertions(+), 1 deletion(-)
diff --git a/sound/soc/rockchip/Kconfig b/sound/soc/rockchip/Kconfig
index b43657e6e655..d610b553ea3b 100644
--- a/sound/soc/rockchip/Kconfig
+++ b/sound/soc/rockchip/Kconfig
@@ -40,9 +40,10 @@ config SND_SOC_ROCKCHIP_MAX98090
select SND_SOC_ROCKCHIP_I2S
select SND_SOC_MAX98090
select SND_SOC_TS3A227E
+ select SND_SOC_HDMI_CODEC
help
Say Y or M here if you want to add support for SoC audio on Rockchip
- boards using the MAX98090 codec, such as Veyron.
+ boards using the MAX98090 codec and HDMI codec, such as Veyron.
config SND_SOC_ROCKCHIP_RT5645
tristate "ASoC support for Rockchip boards using a RT5645/RT5650 codec"
diff --git a/sound/soc/rockchip/rockchip_max98090.c b/sound/soc/rockchip/rockchip_max98090.c
index c82948e383da..c81c4acda917 100644
--- a/sound/soc/rockchip/rockchip_max98090.c
+++ b/sound/soc/rockchip/rockchip_max98090.c
@@ -134,6 +134,25 @@ enum {
DAILINK_HDMI,
};
+static struct snd_soc_jack rk_hdmi_jack;
+
+static int rk_hdmi_init(struct snd_soc_pcm_runtime *runtime)
+{
+ struct snd_soc_card *card = runtime->card;
+ struct snd_soc_component *component = runtime->codec_dai->component;
+ int ret;
+
+ /* enable jack detection */
+ ret = snd_soc_card_jack_new(card, "HDMI Jack", SND_JACK_LINEOUT,
+ &rk_hdmi_jack, NULL, 0);
+ if (ret) {
+ dev_err(card->dev, "Can't new HDMI Jack %d\n", ret);
+ return ret;
+ }
+
+ return hdmi_codec_set_jack_detect(component, &rk_hdmi_jack);
+}
+
/* max98090 and HDMI codec dai_link */
static struct snd_soc_dai_link rk_dailinks[] = {
[DAILINK_MAX98090] = {
@@ -151,6 +170,7 @@ static struct snd_soc_dai_link rk_dailinks[] = {
.ops = &rk_aif1_ops,
.dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF |
SND_SOC_DAIFMT_CBS_CFS,
+ .init = rk_hdmi_init,
SND_SOC_DAILINK_REG(hdmi),
}
};
--
2.23.0.237.gc6a4ce50a0-goog
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [RFC PATCH v3 4/6] psci: Add hvc call service for ptp_kvm.
From: Paolo Bonzini @ 2019-09-18 8:25 UTC (permalink / raw)
To: Jianyong Wu, netdev, yangbo.lu, john.stultz, tglx,
sean.j.christopherson, maz, richardcochran, Mark.Rutland,
Will.Deacon, suzuki.poulose
Cc: justin.he, kvm, Steve.Capper, linux-kernel, Kaly.Xin, nd,
linux-arm-kernel
In-Reply-To: <20190918080716.64242-5-jianyong.wu@arm.com>
On 18/09/19 10:07, Jianyong Wu wrote:
> + case ARM_SMCCC_VENDOR_HYP_KVM_PTP_FUNC_ID:
> + getnstimeofday(ts);
This is not Y2038-safe. Please use ktime_get_real_ts64 instead, and
split the 64-bit seconds value between val[0] and val[1].
However, it seems to me that the new function is not needed and you can
just use ktime_get_snapshot. You'll get the time in
systime_snapshot->real and the cycles value in systime_snapshot->cycles.
> + get_current_counterval(&sc);
> + val[0] = ts->tv_sec;
> + val[1] = ts->tv_nsec;
> + val[2] = sc.cycles;
> + val[3] = 0;
> + break;
This should return a guest-cycles value. If the cycles values always
the same between the host and the guest on ARM, then okay. If not, you
have to apply whatever offset exists.
Thanks,
Paolo
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ 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