Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 0/2] ARM: Add Rockchip rk3288w support
From: Mylène Josserand @ 2020-06-02  8:06 UTC (permalink / raw)
  To: mturquette, sboyd, heiko, robh+dt
  Cc: devicetree, mylene.josserand, linux-kernel, linux-rockchip,
	kernel, linux-clk, linux-arm-kernel

Hello everyone,

Context
-------

Here is my V4 of my patches that add the support for the Rockchip
RK3288w which is a revision of the RK3288. It is mostly the same SOC
except for, at least, one clock tree which is different.
This difference is only known by looking at the BSP kernel [1].

Currently, the mainline kernel will not hang on rk3288w but it is
probably by "chance" because we got an issue on a lower kernel version.

According to Rockchip's U-Boot [2], the rk3288w can be detected using
the HDMI revision number (= 0x1A) in this version of the SOC.

Changelog
---------

This V4 is pretty much the same than the V3. Added the dt-bindings
documentation in clock-controller dt-bindings and fixed some typos
according to Heiko's reviews.

Changes since v3:
   - Updated clock-controller's dt-bindings
   - Fixed indentation

Best regards,
Mylène Josserand

[1] https://github.com/rockchip-linux/kernel/blob/develop-4.4/drivers/clk/rockchip/clk-rk3288.c#L960..L964
[2] https://github.com/rockchip-linux/u-boot/blob/next-dev/arch/arm/mach-rockchip/rk3288/rk3288.c#L378..L388

Mylène Josserand (2):
  clk: rockchip: rk3288: Handle clock tree for rk3288w
  dt-bindings: clocks: rk3288: add possible rk3288w

 .../bindings/clock/rockchip,rk3288-cru.txt    |  8 +++++++-
 drivers/clk/rockchip/clk-rk3288.c             | 20 +++++++++++++++++--
 2 files changed, 25 insertions(+), 3 deletions(-)

-- 
2.26.2


_______________________________________________
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 v5 1/6] dt-bindings: exynos-bus: Add documentation for interconnect properties
From: Krzysztof Kozlowski @ 2020-06-02  8:05 UTC (permalink / raw)
  To: Sylwester Nawrocki
  Cc: linux-samsung-soc, b.zolnierkie, sw0312.kim, a.swigon, dri-devel,
	linux-kernel, inki.dae, cw00.choi, myungjoo.ham, georgi.djakov,
	linux-arm-kernel, m.szyprowski
In-Reply-To: <20200529163200.18031-2-s.nawrocki@samsung.com>

On Fri, May 29, 2020 at 06:31:55PM +0200, Sylwester Nawrocki wrote:
> Add documentation for new optional properties in the exynos bus nodes:
> samsung,interconnect-parent, #interconnect-cells.
> These properties allow to specify the SoC interconnect structure which
> then allows the interconnect consumer devices to request specific
> bandwidth requirements.
> 
> Signed-off-by: Artur Świgoń <a.swigon@samsung.com>
> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
> ---
> Changes for v5:
>  - exynos,interconnect-parent-node renamed to samsung,interconnect-parent
> ---
>  Documentation/devicetree/bindings/devfreq/exynos-bus.txt | 15 +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)

Acked-by: Krzysztof Kozlowski <krzk@kernel.org>

Best regards,
Krzysztof

_______________________________________________
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 3/3] iio: remove iio_triggered_buffer_postenable()/iio_triggered_buffer_predisable()
From: Ardelean, Alexandru @ 2020-06-02  7:50 UTC (permalink / raw)
  To: jic23@kernel.org
  Cc: lars@metafoo.de, s.hauer@pengutronix.de, alexandre.torgue@st.com,
	linux-iio@vger.kernel.org, linus.walleij@linaro.org,
	linux-kernel@vger.kernel.org, songqiang1304521@gmail.com,
	mcoquelin.stm32@gmail.com, lorenzo.bianconi83@gmail.com,
	shawnguo@kernel.org, linux-stm32@st-md-mailman.stormreply.com,
	linux-arm-kernel@lists.infradead.org
In-Reply-To: <20200531164020.765822dc@archlinux>

On Sun, 2020-05-31 at 16:40 +0100, Jonathan Cameron wrote:
> On Mon, 25 May 2020 14:38:55 +0300
> Alexandru Ardelean <alexandru.ardelean@analog.com> wrote:
> 
> > From: Lars-Peter Clausen <lars@metafoo.de>
> > 
> > This patch should be squashed into the first one, as the first one is
> > breaking the build (intentionally) to make the IIO core files easier to
> > review.
> > 
> > Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
> > ---
> 
> Friend poke.  Version log?

Version log is in the first patch.
I was wondering if I omitted it.
Seems, this time I didn't. But I admit, it probably would have been better
here.

> 
> Other than the wistful comment below (which I'm not expecting you to
> do anything about btw!) whole series looks good to me.
> 
> These are obviously no functional changes (I think) so it's only really
> patch 2 that
> could do with more eyes and acks.
> 
> Far as I can tell that case is fine as well because of the protections
> on being in the right mode, but more eyes on that would be great.
> 
> So assuming that's fine, what commit message do you want me to use for
> the fused single patch?

Commit message-wise: I think the message in the first commit would be
mostly sufficient.
No idea what other description would be needed.

So, maybe something like:

----------------------------------------------------------------------
All devices using a triggered buffer need to attach and detach the trigger
to the device in order to properly work. Instead of doing this in each and
every driver by hand move this into the core.

At this point in time, all drivers should have been resolved to
attach/detach the poll-function in the same order.

This patch removes all explicit calls of iio_triggered_buffer_postenable()
& iio_triggered_buffer_predisable() in all drivers, since the core handles
now the pollfunc attach/detach.

The more peculiar change is for the 'at91-sama5d2_adc' driver, since it's
not obvious that removing the hooks doesn't break anything**
----------------------------------------------------------------------

** for the comment about 'at91-sama5d2_adc', we really do need to get some
testing; otherwise this risks breaking it.


> 
> Thanks,
> 
> Jonathan
> 
> >  static const struct iio_trigger_ops atlas_interrupt_trigger_ops = {
> > diff --git a/drivers/iio/dummy/iio_simple_dummy_buffer.c
> > b/drivers/iio/dummy/iio_simple_dummy_buffer.c
> > index 17606eca42b4..8e13c53d4360 100644
> > --- a/drivers/iio/dummy/iio_simple_dummy_buffer.c
> > +++ b/drivers/iio/dummy/iio_simple_dummy_buffer.c
> > @@ -99,20 +99,6 @@ static irqreturn_t iio_simple_dummy_trigger_h(int
> > irq, void *p)
> >  }
> >  
> >  static const struct iio_buffer_setup_ops
> > iio_simple_dummy_buffer_setup_ops = {
> > -	/*
> > -	 * iio_triggered_buffer_postenable:
> > -	 * Generic function that simply attaches the pollfunc to the
> > trigger.
> > -	 * Replace this to mess with hardware state before we attach the
> > -	 * trigger.
> > -	 */
> > -	.postenable = &iio_triggered_buffer_postenable,
> > -	/*
> > -	 * iio_triggered_buffer_predisable:
> > -	 * Generic function that simple detaches the pollfunc from the
> > trigger.
> > -	 * Replace this to put hardware state back again after the trigger
> > is
> > -	 * detached but before userspace knows we have disabled the ring.
> > -	 */
> > -	.predisable = &iio_triggered_buffer_predisable,
> >  };
> >  
> Hmm. Guess we should probably 'invent' a reason to illustrate the bufer
> ops in the dummy example.  Anyone feeling creative?
_______________________________________________
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 2/4] firmware: imx: add resource management api
From: Peng Fan @ 2020-06-02  7:47 UTC (permalink / raw)
  To: Aisheng Dong, shawnguo@kernel.org, s.hauer@pengutronix.de
  Cc: festevam@gmail.com, Joakim Zhang, linux@rempel-privat.de,
	linux-arm-kernel@lists.infradead.org, kernel@pengutronix.de,
	Leonard Crestez, Daniel Baluta, linux-kernel@vger.kernel.org,
	dl-linux-imx
In-Reply-To: <AM6PR04MB4966FC45AF02058F8297FF40808B0@AM6PR04MB4966.eurprd04.prod.outlook.com>

> Subject: RE: [PATCH 2/4] firmware: imx: add resource management api
> 
> > From: Peng Fan <peng.fan@nxp.com>
> > Sent: Tuesday, June 2, 2020 12:51 PM
> > >
> > > > From: Peng Fan <peng.fan@nxp.com>
> > > > Sent: Monday, June 1, 2020 8:40 PM
> > > > >
> > > > > > From: Peng Fan <peng.fan@nxp.com>
> > > > > > Sent: Friday, April 24, 2020 9:12 AM
> > > > > > >
> > > > > > > > From: Peng Fan <peng.fan@nxp.com>
> > > > > > > > Sent: Thursday, April 23, 2020 6:57 PM
> > > > > > > > > > From: Peng Fan <peng.fan@nxp.com>
> > > > > > > > > > Sent: Thursday, April 23, 2020 3:00 PM
> > > > > > > > > >
> > > > > > > > > > Add resource management API, when we have multiple
> > > > > > > > > > partition running together, resources not owned to
> > > > > > > > > > current partition should not be
> > > > > > > used.
> > > > > > > > > >
> > > > > > > > > > Reviewed-by: Leonard Crestez <leonard.crestez@nxp.com>
> > > > > > > > > > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > > > > > > > >
> > > > > > > > > Right now I'm still not quite understand if we really need this.
> > > > > > > > > As current resource is bound to power domains, if it's
> > > > > > > > > not owned by one specific SW execution environment,
> > > > > > > > > power on will also
> > > > fail.
> > > > > > > > > Can you clarify if any exceptions?
> > > > > > > >
> > > > > > > > There will be lots of failures when boot Linux domu if no check.
> > > > > > > >
> > > > > > >
> > > > > > > What kind of features did you mean?
> > > > > > > Could you clarify a bit more? I'd like to understand this issue
> better.
> > > > > >
> > > > > > When supporting hypervisor with dual Linux running, 1st Linux
> > > > > > and the 2nd Linux will have their own dedicated resources.
> > > > > >
> > > > > > If no resource check, that means 1st/2nd Linux will register
> > > > > > all the resources, then both will see fail logs when register
> > > > > > resources not owned to
> > > > > itself.
> > > > > >
> > > > > > Same to partitioned m4 case.
> > > > > >
> > > > > > Would it be better without the failure log?
> > > > > >
> > > > >
> > > > > Is it power up failure?
> > > > > If yes, it's expected because we actually use power up to check
> > > > > if resources are owned by this partition. It functions the same
> > > > > as calling resource check API.
> > > > > That's current design.
> > > > >
> > > > > Or it's other failures? Log?
> > > > Sorry for long late reply.
> > > >
> > > > Part of my XEN DomU log, there are lots of failure log. I think
> > > > better not have such logs by add a resource own check.
> > >
> > > Those error logs are intended.
> > > Resource owner check actually behaves the same as power up.
> >
> > If resource is not owned, it will not register the power domain.
> >
> > Without resource own check, it will continue register the power domain
> > and hook it into genpd list.
> >
> 
> That's the intended behavior to save the resource owner checking as it's very
> time cost to send SCU cmd for each domain during booting which benefits
> nothing except not exposing them in genpd list.
> 
> > I prefer we not expose the power domain not owned to current partition
> > and keep the err msg for people to easy see where it goes wrong.
> 
> If we really want to to do, I wonder probably another better approach is
> trying to re-use the partition Information built by bootloader as uboot already
> did that one time, not necessary to re-do It again for kernel as it's time cost.
> e.g. introduce a resource partition property in dt and initialized and passed by
> bootloarder to kernel to use later.

This will not work for hypervisor based VM runtime partition create and resource
assignment.

> Then we can still save those huge number of resource owner check CMDs.

So we have to live with them even I only need one SDHC for a VM? With
so many clk-scu entries and power domain entries there.

Thanks,
Peng.

> 
> Regards
> Aisheng
> 
> >
> > Regards,
> > Peng.
> > > So not quite necessary to add a double check.
> > > If we're concerning about the error log, we can change it to dev_dbg().
> > >
> > > BTW, as resource management will be needed by seco drivers later, So
> > > I will continue to review the patch.
> > >
> > > Regards
> > > Aisheng
> > >
> > > >
> > > > [    2.034774] imx6q-pcie 5f000000.pcie:    IO
> 0x6ff80000..0x6ff8ffff
> > ->
> > > > 0x00000000
> > > > [    2.034801] imx6q-pcie 5f000000.pcie:   MEM
> > 0x60000000..0x6fefffff
> > > ->
> > > > 0x60000000
> > > > [    2.035072]  sdhc1: failed to power up resource 249 ret -13
> > > > [    2.035619]  sdhc2: failed to power up resource 250 ret -13
> > > > [    2.036126]  enet0: failed to power up resource 251 ret -13
> > > > [    2.036584]  enet1: failed to power up resource 252 ret -13
> > > > [    2.037040]  mlb0: failed to power up resource 253 ret -13
> > > > [    2.037495]  nand: failed to power up resource 265 ret -13
> > > > [    2.037951]  nand: failed to power up resource 265 ret -13
> > > > [    2.038416]  pwm0: failed to power up resource 191 ret -13
> > > > [    2.038868]  pwm1: failed to power up resource 192 ret -13
> > > > [    2.039320]  pwm2: failed to power up resource 193 ret -13
> > > > [    2.039786]  pwm3: failed to power up resource 194 ret -13
> > > > [    2.040239]  pwm4: failed to power up resource 195 ret -13
> > > > [    2.040692]  pwm5: failed to power up resource 196 ret -13
> > > > [    2.041142]  pwm6: failed to power up resource 197 ret -13
> > > > [    2.041596]  pwm7: failed to power up resource 198 ret -13
> > > > [    2.042073]  amix: failed to power up resource 458 ret -13
> > > > [    2.042558]  lpspi0: failed to power up resource 53 ret -13
> > > > [    2.043033]  lpspi1: failed to power up resource 54 ret -13
> > > > [    2.043501]  lpspi2: failed to power up resource 55 ret -13
> > > > [    2.043992]  lpspi3: failed to power up resource 56 ret -13
> > > > [    2.044460]  lpuart0: failed to power up resource 57 ret -13
> > > > [    2.044935]  lpuart2: failed to power up resource 59 ret -13
> > > > [    2.045409]  lpuart3: failed to power up resource 60 ret -13
> > > > [    2.055014]  sim0: failed to power up resource 62 ret -13
> > > > [    2.055510]  adc0: failed to power up resource 101 ret -13
> > > > [    2.056467]  lpi2c0: failed to power up resource 96 ret -13
> > > > [    2.056946]  lpi2c1: failed to power up resource 97 ret -13
> > > > [    2.057424]  lpi2c2: failed to power up resource 98 ret -13
> > > > [    2.057898]  lpi2c3: failed to power up resource 99 ret -13
> > > > [    2.058371]  can0: failed to power up resource 105 ret -13
> > > > [    2.059289]  can1: failed to power up resource 106 ret -13
> > > > [    2.059801]  can2: failed to power up resource 107 ret -13
> > > > [    2.060281]  nand: failed to power up resource 265 ret -13
> > > > [    2.062764] dpu-core 56180000.dpu: driver probed
> > > >
> > > > Thanks,
> > > > Peng.
> > > >
> > > > >
> > > > > Regards
> > > > > Aisheng
> > > > >
> > > > > > Thanks,
> > > > > > Peng.
> > > > > >
> > > > > >
> > > > > > >
> > > > > > > Regards
> > > > > > > Aisheng
> > > > > > >
> > > > > > > > Thanks,
> > > > > > > > Peng.
> > > > > > > >
> > > > > > > > >
> > > > > > > > > Regards
> > > > > > > > > Aisheng
> > > > > > > > >
> > > > > > > > > > ---
> > > > > > > > > >  drivers/firmware/imx/Makefile       |  2 +-
> > > > > > > > > >  drivers/firmware/imx/rm.c           | 40
> > > > > > > +++++++++++++++++++++
> > > > > > > > > >  include/linux/firmware/imx/sci.h    |  1 +
> > > > > > > > > >  include/linux/firmware/imx/svc/rm.h | 69
> > > > > > > > > > +++++++++++++++++++++++++++++++++++++
> > > > > > > > > >  4 files changed, 111 insertions(+), 1 deletion(-)
> > > > > > > > > > create mode
> > > > > > > > > > 100644 drivers/firmware/imx/rm.c  create mode 100644
> > > > > > > > > > include/linux/firmware/imx/svc/rm.h
> > > > > > > > > >
> > > > > > > > > > diff --git a/drivers/firmware/imx/Makefile
> > > > > > > > > > b/drivers/firmware/imx/Makefile index
> > > > > > > > > > 08bc9ddfbdfb..17ea3613e142
> > > > > > > > > > 100644
> > > > > > > > > > --- a/drivers/firmware/imx/Makefile
> > > > > > > > > > +++ b/drivers/firmware/imx/Makefile
> > > > > > > > > > @@ -1,4 +1,4 @@
> > > > > > > > > >  # SPDX-License-Identifier: GPL-2.0
> > > > > > > > > >  obj-$(CONFIG_IMX_DSP)		+= imx-dsp.o
> > > > > > > > > > -obj-$(CONFIG_IMX_SCU)		+= imx-scu.o misc.o
> > imx-scu-irq.o
> > > > > > > > > > +obj-$(CONFIG_IMX_SCU)		+= imx-scu.o misc.o
> > > imx-scu-irq.o
> > > > > rm.o
> > > > > > > > > >  obj-$(CONFIG_IMX_SCU_PD)	+= scu-pd.o
> > > > > > > > > > diff --git a/drivers/firmware/imx/rm.c
> > > > > > > > > > b/drivers/firmware/imx/rm.c new file mode 100644 index
> > > > > > > > > > 000000000000..7b0334de5486
> > > > > > > > > > --- /dev/null
> > > > > > > > > > +++ b/drivers/firmware/imx/rm.c
> > > > > > > > > > @@ -0,0 +1,40 @@
> > > > > > > > > > +// SPDX-License-Identifier: GPL-2.0+
> > > > > > > > > > +/*
> > > > > > > > > > + * Copyright 2020 NXP
> > > > > > > > > > + *
> > > > > > > > > > + * File containing client-side RPC functions for the RM
> service.
> > > > > > > > > > +These
> > > > > > > > > > + * function are ported to clients that communicate to the SC.
> > > > > > > > > > + */
> > > > > > > > > > +
> > > > > > > > > > +#include <linux/firmware/imx/svc/rm.h>
> > > > > > > > > > +
> > > > > > > > > > +struct imx_sc_msg_rm_rsrc_owned {
> > > > > > > > > > +	struct imx_sc_rpc_msg hdr;
> > > > > > > > > > +	u16 resource;
> > > > > > > > > > +} __packed __aligned(4);
> > > > > > > > > > +
> > > > > > > > > > +/*
> > > > > > > > > > + * This function check @resource is owned by current
> > > > > > > > > > +partition or not
> > > > > > > > > > + *
> > > > > > > > > > + * @param[in]     ipc         IPC handle
> > > > > > > > > > + * @param[in]     resource    resource the control is
> > > > > associated
> > > > > > > with
> > > > > > > > > > + *
> > > > > > > > > > + * @return Returns 0 for success and < 0 for errors.
> > > > > > > > > > + */
> > > > > > > > > > +bool imx_sc_rm_is_resource_owned(struct imx_sc_ipc
> > > > > > > > > > +*ipc,
> > > > > > > > > > +u16
> > > > > > > > > > +resource) {
> > > > > > > > > > +	struct imx_sc_msg_rm_rsrc_owned msg;
> > > > > > > > > > +	struct imx_sc_rpc_msg *hdr = &msg.hdr;
> > > > > > > > > > +
> > > > > > > > > > +	hdr->ver = IMX_SC_RPC_VERSION;
> > > > > > > > > > +	hdr->svc = IMX_SC_RPC_SVC_RM;
> > > > > > > > > > +	hdr->func = IMX_SC_RM_FUNC_IS_RESOURCE_OWNED;
> > > > > > > > > > +	hdr->size = 2;
> > > > > > > > > > +
> > > > > > > > > > +	msg.resource = resource;
> > > > > > > > > > +
> > > > > > > > > > +	imx_scu_call_rpc(ipc, &msg, true);
> > > > > > > > > > +
> > > > > > > > > > +	return hdr->func;
> > > > > > > > > > +}
> > > > > > > > > > +EXPORT_SYMBOL(imx_sc_rm_is_resource_owned);
> > > > > > > > > > diff --git a/include/linux/firmware/imx/sci.h
> > > > > > > > > > b/include/linux/firmware/imx/sci.h
> > > > > > > > > > index 17ba4e405129..b5c5a56f29be 100644
> > > > > > > > > > --- a/include/linux/firmware/imx/sci.h
> > > > > > > > > > +++ b/include/linux/firmware/imx/sci.h
> > > > > > > > > > @@ -15,6 +15,7 @@
> > > > > > > > > >
> > > > > > > > > >  #include <linux/firmware/imx/svc/misc.h>  #include
> > > > > > > > > > <linux/firmware/imx/svc/pm.h>
> > > > > > > > > > +#include <linux/firmware/imx/svc/rm.h>
> > > > > > > > > >
> > > > > > > > > >  int imx_scu_enable_general_irq_channel(struct device
> > > > > > > > > > *dev); int imx_scu_irq_register_notifier(struct
> > > > > > > > > > notifier_block *nb); diff --git
> > > > > > > > > > a/include/linux/firmware/imx/svc/rm.h
> > > > > > > > > > b/include/linux/firmware/imx/svc/rm.h
> > > > > > > > > > new file mode 100644
> > > > > > > > > > index 000000000000..fc6ea62d9d0e
> > > > > > > > > > --- /dev/null
> > > > > > > > > > +++ b/include/linux/firmware/imx/svc/rm.h
> > > > > > > > > > @@ -0,0 +1,69 @@
> > > > > > > > > > +/* SPDX-License-Identifier: GPL-2.0+ */
> > > > > > > > > > +/*
> > > > > > > > > > + * Copyright (C) 2016 Freescale Semiconductor, Inc.
> > > > > > > > > > + * Copyright 2017-2019 NXP
> > > > > > > > > > + *
> > > > > > > > > > + * Header file containing the public API for the
> > > > > > > > > > +System Controller
> > > > > > > > > > +(SC)
> > > > > > > > > > + * Power Management (PM) function. This includes
> > > > > > > > > > +functions for power state
> > > > > > > > > > + * control, clock control, reset control, and wake-up
> > > > > > > > > > +event
> > > control.
> > > > > > > > > > + *
> > > > > > > > > > + * RM_SVC (SVC) Resource Management Service
> > > > > > > > > > + *
> > > > > > > > > > + * Module for the Resource Management (RM) service.
> > > > > > > > > > + */
> > > > > > > > > > +
> > > > > > > > > > +#ifndef _SC_RM_API_H
> > > > > > > > > > +#define _SC_RM_API_H
> > > > > > > > > > +
> > > > > > > > > > +#include <linux/firmware/imx/sci.h>
> > > > > > > > > > +
> > > > > > > > > > +/*
> > > > > > > > > > + * This type is used to indicate RPC RM function calls.
> > > > > > > > > > + */
> > > > > > > > > > +enum imx_sc_rm_func {
> > > > > > > > > > +	IMX_SC_RM_FUNC_UNKNOWN = 0,
> > > > > > > > > > +	IMX_SC_RM_FUNC_PARTITION_ALLOC = 1,
> > > > > > > > > > +	IMX_SC_RM_FUNC_SET_CONFIDENTIAL = 31,
> > > > > > > > > > +	IMX_SC_RM_FUNC_PARTITION_FREE = 2,
> > > > > > > > > > +	IMX_SC_RM_FUNC_GET_DID = 26,
> > > > > > > > > > +	IMX_SC_RM_FUNC_PARTITION_STATIC = 3,
> > > > > > > > > > +	IMX_SC_RM_FUNC_PARTITION_LOCK = 4,
> > > > > > > > > > +	IMX_SC_RM_FUNC_GET_PARTITION = 5,
> > > > > > > > > > +	IMX_SC_RM_FUNC_SET_PARENT = 6,
> > > > > > > > > > +	IMX_SC_RM_FUNC_MOVE_ALL = 7,
> > > > > > > > > > +	IMX_SC_RM_FUNC_ASSIGN_RESOURCE = 8,
> > > > > > > > > > +	IMX_SC_RM_FUNC_SET_RESOURCE_MOVABLE = 9,
> > > > > > > > > > +	IMX_SC_RM_FUNC_SET_SUBSYS_RSRC_MOVABLE = 28,
> > > > > > > > > > +	IMX_SC_RM_FUNC_SET_MASTER_ATTRIBUTES = 10,
> > > > > > > > > > +	IMX_SC_RM_FUNC_SET_MASTER_SID = 11,
> > > > > > > > > > +	IMX_SC_RM_FUNC_SET_PERIPHERAL_PERMISSIONS =
> 12,
> > > > > > > > > > +	IMX_SC_RM_FUNC_IS_RESOURCE_OWNED = 13,
> > > > > > > > > > +	IMX_SC_RM_FUNC_GET_RESOURCE_OWNER = 33,
> > > > > > > > > > +	IMX_SC_RM_FUNC_IS_RESOURCE_MASTER = 14,
> > > > > > > > > > +	IMX_SC_RM_FUNC_IS_RESOURCE_PERIPHERAL = 15,
> > > > > > > > > > +	IMX_SC_RM_FUNC_GET_RESOURCE_INFO = 16,
> > > > > > > > > > +	IMX_SC_RM_FUNC_MEMREG_ALLOC = 17,
> > > > > > > > > > +	IMX_SC_RM_FUNC_MEMREG_SPLIT = 29,
> > > > > > > > > > +	IMX_SC_RM_FUNC_MEMREG_FRAG = 32,
> > > > > > > > > > +	IMX_SC_RM_FUNC_MEMREG_FREE = 18,
> > > > > > > > > > +	IMX_SC_RM_FUNC_FIND_MEMREG = 30,
> > > > > > > > > > +	IMX_SC_RM_FUNC_ASSIGN_MEMREG = 19,
> > > > > > > > > > +	IMX_SC_RM_FUNC_SET_MEMREG_PERMISSIONS = 20,
> > > > > > > > > > +	IMX_SC_RM_FUNC_IS_MEMREG_OWNED = 21,
> > > > > > > > > > +	IMX_SC_RM_FUNC_GET_MEMREG_INFO = 22,
> > > > > > > > > > +	IMX_SC_RM_FUNC_ASSIGN_PAD = 23,
> > > > > > > > > > +	IMX_SC_RM_FUNC_SET_PAD_MOVABLE = 24,
> > > > > > > > > > +	IMX_SC_RM_FUNC_IS_PAD_OWNED = 25,
> > > > > > > > > > +	IMX_SC_RM_FUNC_DUMP = 27, };
> > > > > > > > > > +
> > > > > > > > > > +#if IS_ENABLED(CONFIG_IMX_SCU) bool
> > > > > > > > > > +imx_sc_rm_is_resource_owned(struct imx_sc_ipc *ipc,
> > > > > > > > > > +u16 resource); #else static inline bool
> > > > > > > > > > +imx_sc_rm_is_resource_owned(struct imx_sc_ipc *ipc,
> > > > > > > > > > +u16
> > > > > resource) {
> > > > > > > > > > +	return true;
> > > > > > > > > > +}
> > > > > > > > > > +#endif
> > > > > > > > > > +#endif
> > > > > > > > > > --
> > > > > > > > > > 2.16.4

_______________________________________________
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 2/2] coresight: tmc: Add shutdown callback for TMC ETR/ETF
From: Sai Prakash Ranjan @ 2020-06-02  7:30 UTC (permalink / raw)
  To: Mathieu Poirier
  Cc: Suzuki K Poulose, linux-arm-msm, coresight, linux-kernel,
	Stephen Boyd, robin.murphy, linux-arm-kernel, Mike Leach
In-Reply-To: <20200601212858.GB24287@xps15>

Hi Mathieu,

Thanks for taking your time for review.

On 2020-06-02 02:58, Mathieu Poirier wrote:
> Hi Sai,
> 
> On top of the comments already privided by Mike, I have the following:
> 
> On Mon, Jun 01, 2020 at 01:32:26PM +0530, Sai Prakash Ranjan wrote:
>> Implement a shutdown callback to ensure ETR/ETF hardware is
>> properly shutdown in reboot/shutdown path. This is required
>> for ETR/ETF which has SMMU address translation enabled like
>> on SC7180 SoC and few others. If the hardware is still accessing
>> memory after SMMU translation is disabled as part of SMMU
>> shutdown callback in system reboot or shutdown path, then
>> IOVAs(I/O virtual address) which it was using will go on the bus
>> as the physical addresses which might result in unknown crashes
>> (NoC/interconnect errors). So we make sure from this shutdown
>> callback that the ETR/ETF is shutdown before SMMU translation is
>> disabled and device_link in SMMU driver will take care of ordering
>> of shutdown callbacks such that SMMU shutdown callback is not
>> called before any of its consumer shutdown callbacks.
>> 
>> Signed-off-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>
>> ---
>>  .../hwtracing/coresight/coresight-tmc-etf.c   |  4 +--
>>  .../hwtracing/coresight/coresight-tmc-etr.c   |  2 +-
>>  drivers/hwtracing/coresight/coresight-tmc.c   | 29 
>> +++++++++++++++++++
>>  drivers/hwtracing/coresight/coresight-tmc.h   |  3 ++
>>  4 files changed, 35 insertions(+), 3 deletions(-)
>> 
>> diff --git a/drivers/hwtracing/coresight/coresight-tmc-etf.c 
>> b/drivers/hwtracing/coresight/coresight-tmc-etf.c
>> index 36cce2bfb744..cba3e7592820 100644
>> --- a/drivers/hwtracing/coresight/coresight-tmc-etf.c
>> +++ b/drivers/hwtracing/coresight/coresight-tmc-etf.c
>> @@ -85,7 +85,7 @@ static void __tmc_etb_disable_hw(struct tmc_drvdata 
>> *drvdata)
>>  	CS_LOCK(drvdata->base);
>>  }
>> 
>> -static void tmc_etb_disable_hw(struct tmc_drvdata *drvdata)
>> +void tmc_etb_disable_hw(struct tmc_drvdata *drvdata)
>>  {
>>  	__tmc_etb_disable_hw(drvdata);
>>  	coresight_disclaim_device(drvdata->base);
>> @@ -118,7 +118,7 @@ static int tmc_etf_enable_hw(struct tmc_drvdata 
>> *drvdata)
>>  	return 0;
>>  }
>> 
>> -static void tmc_etf_disable_hw(struct tmc_drvdata *drvdata)
>> +void tmc_etf_disable_hw(struct tmc_drvdata *drvdata)
>>  {
>>  	CS_UNLOCK(drvdata->base);
>> 
> 
> Why do we care about ETB and ETF when they both use RAM internal to the 
> device?
> Moreover, the system RAM they use is not dedicated and as such falls 
> back to the
> kernel's memory pool.
> 

Actually we don't, I added the disable for ETF/ETB for completeness 
since we are
adding shutdown callback for TMC devices and not just ETR although this 
issue applies
only for ETR and it doesn't hurt to disable these devices in shutdown 
path.

>> diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c 
>> b/drivers/hwtracing/coresight/coresight-tmc-etr.c
>> index 625882bc8b08..b29c2db94d96 100644
>> --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
>> +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
>> @@ -1110,7 +1110,7 @@ static void __tmc_etr_disable_hw(struct 
>> tmc_drvdata *drvdata)
>> 
>>  }
>> 
>> -static void tmc_etr_disable_hw(struct tmc_drvdata *drvdata)
>> +void tmc_etr_disable_hw(struct tmc_drvdata *drvdata)
>>  {
>>  	__tmc_etr_disable_hw(drvdata);
>>  	/* Disable CATU device if this ETR is connected to one */
>> diff --git a/drivers/hwtracing/coresight/coresight-tmc.c 
>> b/drivers/hwtracing/coresight/coresight-tmc.c
>> index 5a271ebc4585..7e687a356fe0 100644
>> --- a/drivers/hwtracing/coresight/coresight-tmc.c
>> +++ b/drivers/hwtracing/coresight/coresight-tmc.c
>> @@ -540,6 +540,34 @@ static int tmc_probe(struct amba_device *adev, 
>> const struct amba_id *id)
>>  	return ret;
>>  }
>> 
>> +static void tmc_shutdown(struct amba_device *adev)
>> +{
>> +	struct tmc_drvdata *drvdata = amba_get_drvdata(adev);
>> +
>> +	if (!drvdata->enable)
>> +		goto out;
>> +
>> +	/*
>> +	 * We do not care about the active trace sessions here
>> +	 * since the system is going down unlike remove callback,
>> +	 * just make sure that the hardware is shutdown.
>> +	 */
>> +	switch (drvdata->config_type) {
>> +	case TMC_CONFIG_TYPE_ETB:
>> +		tmc_etb_disable_hw(drvdata);
>> +		break;
>> +	case TMC_CONFIG_TYPE_ETF:
>> +		tmc_etf_disable_hw(drvdata);
>> +		break;
>> +	case TMC_CONFIG_TYPE_ETR:
>> +		tmc_etr_disable_hw(drvdata);
>> +	}
>> +
>> +out:
>> +	misc_deregister(&drvdata->miscdev);
>> +	coresight_unregister(drvdata->csdev);
> 
> If a session is active when tmc_shutdown() is called, unregistering the 
> ETF/ETR
> will result in a kernel crash if the session is stopped before the 
> kernel has
> had the opportunity to shutdown.  It is the problem as trying to make 
> coresight
> drivers modular.
> 
> For this to really work the ongoing session would need to be stopped.  
> That
> would teardown the path and stop the sink.

I have tested this with and without active trace sessions multiple times 
on 2 devices
and did not observe a single crash. The crash should be easily triggered 
as per
what you are saying if we have active sessions but I do not see any 
crash.

> 
> That being said I'm sure that dependencies on an IOMMU isn't a problem 
> confined
> to coresight. I am adding Robin Murphy, who added this commit [1], to 
> the thread
> in the hope that he can provide guidance on the right way to do this.
> 

SMMU/IOMMU won't be able to do much here as it is the client's 
responsiblity to
properly shutdown and SMMU device link just makes sure that 
SMMU(supplier) shutdown is
called only after its consumers shutdown callbacks are called.

Thanks,
Sai

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a 
member
of Code Aurora Forum, hosted by The Linux Foundation

_______________________________________________
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 2/4] firmware: imx: add resource management api
From: Aisheng Dong @ 2020-06-02  7:28 UTC (permalink / raw)
  To: Peng Fan, shawnguo@kernel.org, s.hauer@pengutronix.de
  Cc: festevam@gmail.com, Joakim Zhang, linux@rempel-privat.de,
	linux-arm-kernel@lists.infradead.org, kernel@pengutronix.de,
	Leonard Crestez, Daniel Baluta, linux-kernel@vger.kernel.org,
	dl-linux-imx
In-Reply-To: <DB6PR0402MB276061C15AC205604BDACC16888B0@DB6PR0402MB2760.eurprd04.prod.outlook.com>

> From: Peng Fan <peng.fan@nxp.com>
> Sent: Tuesday, June 2, 2020 12:51 PM
> >
> > > From: Peng Fan <peng.fan@nxp.com>
> > > Sent: Monday, June 1, 2020 8:40 PM
> > > >
> > > > > From: Peng Fan <peng.fan@nxp.com>
> > > > > Sent: Friday, April 24, 2020 9:12 AM
> > > > > >
> > > > > > > From: Peng Fan <peng.fan@nxp.com>
> > > > > > > Sent: Thursday, April 23, 2020 6:57 PM
> > > > > > > > > From: Peng Fan <peng.fan@nxp.com>
> > > > > > > > > Sent: Thursday, April 23, 2020 3:00 PM
> > > > > > > > >
> > > > > > > > > Add resource management API, when we have multiple
> > > > > > > > > partition running together, resources not owned to
> > > > > > > > > current partition should not be
> > > > > > used.
> > > > > > > > >
> > > > > > > > > Reviewed-by: Leonard Crestez <leonard.crestez@nxp.com>
> > > > > > > > > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > > > > > > >
> > > > > > > > Right now I'm still not quite understand if we really need this.
> > > > > > > > As current resource is bound to power domains, if it's not
> > > > > > > > owned by one specific SW execution environment, power on
> > > > > > > > will also
> > > fail.
> > > > > > > > Can you clarify if any exceptions?
> > > > > > >
> > > > > > > There will be lots of failures when boot Linux domu if no check.
> > > > > > >
> > > > > >
> > > > > > What kind of features did you mean?
> > > > > > Could you clarify a bit more? I'd like to understand this issue better.
> > > > >
> > > > > When supporting hypervisor with dual Linux running, 1st Linux
> > > > > and the 2nd Linux will have their own dedicated resources.
> > > > >
> > > > > If no resource check, that means 1st/2nd Linux will register all
> > > > > the resources, then both will see fail logs when register
> > > > > resources not owned to
> > > > itself.
> > > > >
> > > > > Same to partitioned m4 case.
> > > > >
> > > > > Would it be better without the failure log?
> > > > >
> > > >
> > > > Is it power up failure?
> > > > If yes, it's expected because we actually use power up to check if
> > > > resources are owned by this partition. It functions the same as
> > > > calling resource check API.
> > > > That's current design.
> > > >
> > > > Or it's other failures? Log?
> > > Sorry for long late reply.
> > >
> > > Part of my XEN DomU log, there are lots of failure log. I think
> > > better not have such logs by add a resource own check.
> >
> > Those error logs are intended.
> > Resource owner check actually behaves the same as power up.
> 
> If resource is not owned, it will not register the power domain.
> 
> Without resource own check, it will continue register the power domain and
> hook it into genpd list.
> 

That's the intended behavior to save the resource owner checking as it's very time cost
to send SCU cmd for each domain during booting which benefits nothing except not exposing
them in genpd list.

> I prefer we not expose the power domain not owned to current partition and
> keep the err msg for people to easy see where it goes wrong.

If we really want to to do, I wonder probably another better approach is trying to re-use the partition
Information built by bootloader as uboot already did that one time, not necessary to re-do
It again for kernel as it's time cost.
e.g. introduce a resource partition property in dt and initialized and passed by bootloarder
to kernel to use later.
Then we can still save those huge number of resource owner check CMDs.

Regards
Aisheng

> 
> Regards,
> Peng.
> > So not quite necessary to add a double check.
> > If we're concerning about the error log, we can change it to dev_dbg().
> >
> > BTW, as resource management will be needed by seco drivers later, So I
> > will continue to review the patch.
> >
> > Regards
> > Aisheng
> >
> > >
> > > [    2.034774] imx6q-pcie 5f000000.pcie:    IO 0x6ff80000..0x6ff8ffff
> ->
> > > 0x00000000
> > > [    2.034801] imx6q-pcie 5f000000.pcie:   MEM
> 0x60000000..0x6fefffff
> > ->
> > > 0x60000000
> > > [    2.035072]  sdhc1: failed to power up resource 249 ret -13
> > > [    2.035619]  sdhc2: failed to power up resource 250 ret -13
> > > [    2.036126]  enet0: failed to power up resource 251 ret -13
> > > [    2.036584]  enet1: failed to power up resource 252 ret -13
> > > [    2.037040]  mlb0: failed to power up resource 253 ret -13
> > > [    2.037495]  nand: failed to power up resource 265 ret -13
> > > [    2.037951]  nand: failed to power up resource 265 ret -13
> > > [    2.038416]  pwm0: failed to power up resource 191 ret -13
> > > [    2.038868]  pwm1: failed to power up resource 192 ret -13
> > > [    2.039320]  pwm2: failed to power up resource 193 ret -13
> > > [    2.039786]  pwm3: failed to power up resource 194 ret -13
> > > [    2.040239]  pwm4: failed to power up resource 195 ret -13
> > > [    2.040692]  pwm5: failed to power up resource 196 ret -13
> > > [    2.041142]  pwm6: failed to power up resource 197 ret -13
> > > [    2.041596]  pwm7: failed to power up resource 198 ret -13
> > > [    2.042073]  amix: failed to power up resource 458 ret -13
> > > [    2.042558]  lpspi0: failed to power up resource 53 ret -13
> > > [    2.043033]  lpspi1: failed to power up resource 54 ret -13
> > > [    2.043501]  lpspi2: failed to power up resource 55 ret -13
> > > [    2.043992]  lpspi3: failed to power up resource 56 ret -13
> > > [    2.044460]  lpuart0: failed to power up resource 57 ret -13
> > > [    2.044935]  lpuart2: failed to power up resource 59 ret -13
> > > [    2.045409]  lpuart3: failed to power up resource 60 ret -13
> > > [    2.055014]  sim0: failed to power up resource 62 ret -13
> > > [    2.055510]  adc0: failed to power up resource 101 ret -13
> > > [    2.056467]  lpi2c0: failed to power up resource 96 ret -13
> > > [    2.056946]  lpi2c1: failed to power up resource 97 ret -13
> > > [    2.057424]  lpi2c2: failed to power up resource 98 ret -13
> > > [    2.057898]  lpi2c3: failed to power up resource 99 ret -13
> > > [    2.058371]  can0: failed to power up resource 105 ret -13
> > > [    2.059289]  can1: failed to power up resource 106 ret -13
> > > [    2.059801]  can2: failed to power up resource 107 ret -13
> > > [    2.060281]  nand: failed to power up resource 265 ret -13
> > > [    2.062764] dpu-core 56180000.dpu: driver probed
> > >
> > > Thanks,
> > > Peng.
> > >
> > > >
> > > > Regards
> > > > Aisheng
> > > >
> > > > > Thanks,
> > > > > Peng.
> > > > >
> > > > >
> > > > > >
> > > > > > Regards
> > > > > > Aisheng
> > > > > >
> > > > > > > Thanks,
> > > > > > > Peng.
> > > > > > >
> > > > > > > >
> > > > > > > > Regards
> > > > > > > > Aisheng
> > > > > > > >
> > > > > > > > > ---
> > > > > > > > >  drivers/firmware/imx/Makefile       |  2 +-
> > > > > > > > >  drivers/firmware/imx/rm.c           | 40
> > > > > > +++++++++++++++++++++
> > > > > > > > >  include/linux/firmware/imx/sci.h    |  1 +
> > > > > > > > >  include/linux/firmware/imx/svc/rm.h | 69
> > > > > > > > > +++++++++++++++++++++++++++++++++++++
> > > > > > > > >  4 files changed, 111 insertions(+), 1 deletion(-)
> > > > > > > > > create mode
> > > > > > > > > 100644 drivers/firmware/imx/rm.c  create mode 100644
> > > > > > > > > include/linux/firmware/imx/svc/rm.h
> > > > > > > > >
> > > > > > > > > diff --git a/drivers/firmware/imx/Makefile
> > > > > > > > > b/drivers/firmware/imx/Makefile index
> > > > > > > > > 08bc9ddfbdfb..17ea3613e142
> > > > > > > > > 100644
> > > > > > > > > --- a/drivers/firmware/imx/Makefile
> > > > > > > > > +++ b/drivers/firmware/imx/Makefile
> > > > > > > > > @@ -1,4 +1,4 @@
> > > > > > > > >  # SPDX-License-Identifier: GPL-2.0
> > > > > > > > >  obj-$(CONFIG_IMX_DSP)		+= imx-dsp.o
> > > > > > > > > -obj-$(CONFIG_IMX_SCU)		+= imx-scu.o misc.o
> imx-scu-irq.o
> > > > > > > > > +obj-$(CONFIG_IMX_SCU)		+= imx-scu.o misc.o
> > imx-scu-irq.o
> > > > rm.o
> > > > > > > > >  obj-$(CONFIG_IMX_SCU_PD)	+= scu-pd.o
> > > > > > > > > diff --git a/drivers/firmware/imx/rm.c
> > > > > > > > > b/drivers/firmware/imx/rm.c new file mode 100644 index
> > > > > > > > > 000000000000..7b0334de5486
> > > > > > > > > --- /dev/null
> > > > > > > > > +++ b/drivers/firmware/imx/rm.c
> > > > > > > > > @@ -0,0 +1,40 @@
> > > > > > > > > +// SPDX-License-Identifier: GPL-2.0+
> > > > > > > > > +/*
> > > > > > > > > + * Copyright 2020 NXP
> > > > > > > > > + *
> > > > > > > > > + * File containing client-side RPC functions for the RM service.
> > > > > > > > > +These
> > > > > > > > > + * function are ported to clients that communicate to the SC.
> > > > > > > > > + */
> > > > > > > > > +
> > > > > > > > > +#include <linux/firmware/imx/svc/rm.h>
> > > > > > > > > +
> > > > > > > > > +struct imx_sc_msg_rm_rsrc_owned {
> > > > > > > > > +	struct imx_sc_rpc_msg hdr;
> > > > > > > > > +	u16 resource;
> > > > > > > > > +} __packed __aligned(4);
> > > > > > > > > +
> > > > > > > > > +/*
> > > > > > > > > + * This function check @resource is owned by current
> > > > > > > > > +partition or not
> > > > > > > > > + *
> > > > > > > > > + * @param[in]     ipc         IPC handle
> > > > > > > > > + * @param[in]     resource    resource the control is
> > > > associated
> > > > > > with
> > > > > > > > > + *
> > > > > > > > > + * @return Returns 0 for success and < 0 for errors.
> > > > > > > > > + */
> > > > > > > > > +bool imx_sc_rm_is_resource_owned(struct imx_sc_ipc
> > > > > > > > > +*ipc,
> > > > > > > > > +u16
> > > > > > > > > +resource) {
> > > > > > > > > +	struct imx_sc_msg_rm_rsrc_owned msg;
> > > > > > > > > +	struct imx_sc_rpc_msg *hdr = &msg.hdr;
> > > > > > > > > +
> > > > > > > > > +	hdr->ver = IMX_SC_RPC_VERSION;
> > > > > > > > > +	hdr->svc = IMX_SC_RPC_SVC_RM;
> > > > > > > > > +	hdr->func = IMX_SC_RM_FUNC_IS_RESOURCE_OWNED;
> > > > > > > > > +	hdr->size = 2;
> > > > > > > > > +
> > > > > > > > > +	msg.resource = resource;
> > > > > > > > > +
> > > > > > > > > +	imx_scu_call_rpc(ipc, &msg, true);
> > > > > > > > > +
> > > > > > > > > +	return hdr->func;
> > > > > > > > > +}
> > > > > > > > > +EXPORT_SYMBOL(imx_sc_rm_is_resource_owned);
> > > > > > > > > diff --git a/include/linux/firmware/imx/sci.h
> > > > > > > > > b/include/linux/firmware/imx/sci.h
> > > > > > > > > index 17ba4e405129..b5c5a56f29be 100644
> > > > > > > > > --- a/include/linux/firmware/imx/sci.h
> > > > > > > > > +++ b/include/linux/firmware/imx/sci.h
> > > > > > > > > @@ -15,6 +15,7 @@
> > > > > > > > >
> > > > > > > > >  #include <linux/firmware/imx/svc/misc.h>  #include
> > > > > > > > > <linux/firmware/imx/svc/pm.h>
> > > > > > > > > +#include <linux/firmware/imx/svc/rm.h>
> > > > > > > > >
> > > > > > > > >  int imx_scu_enable_general_irq_channel(struct device
> > > > > > > > > *dev); int imx_scu_irq_register_notifier(struct
> > > > > > > > > notifier_block *nb); diff --git
> > > > > > > > > a/include/linux/firmware/imx/svc/rm.h
> > > > > > > > > b/include/linux/firmware/imx/svc/rm.h
> > > > > > > > > new file mode 100644
> > > > > > > > > index 000000000000..fc6ea62d9d0e
> > > > > > > > > --- /dev/null
> > > > > > > > > +++ b/include/linux/firmware/imx/svc/rm.h
> > > > > > > > > @@ -0,0 +1,69 @@
> > > > > > > > > +/* SPDX-License-Identifier: GPL-2.0+ */
> > > > > > > > > +/*
> > > > > > > > > + * Copyright (C) 2016 Freescale Semiconductor, Inc.
> > > > > > > > > + * Copyright 2017-2019 NXP
> > > > > > > > > + *
> > > > > > > > > + * Header file containing the public API for the System
> > > > > > > > > +Controller
> > > > > > > > > +(SC)
> > > > > > > > > + * Power Management (PM) function. This includes
> > > > > > > > > +functions for power state
> > > > > > > > > + * control, clock control, reset control, and wake-up
> > > > > > > > > +event
> > control.
> > > > > > > > > + *
> > > > > > > > > + * RM_SVC (SVC) Resource Management Service
> > > > > > > > > + *
> > > > > > > > > + * Module for the Resource Management (RM) service.
> > > > > > > > > + */
> > > > > > > > > +
> > > > > > > > > +#ifndef _SC_RM_API_H
> > > > > > > > > +#define _SC_RM_API_H
> > > > > > > > > +
> > > > > > > > > +#include <linux/firmware/imx/sci.h>
> > > > > > > > > +
> > > > > > > > > +/*
> > > > > > > > > + * This type is used to indicate RPC RM function calls.
> > > > > > > > > + */
> > > > > > > > > +enum imx_sc_rm_func {
> > > > > > > > > +	IMX_SC_RM_FUNC_UNKNOWN = 0,
> > > > > > > > > +	IMX_SC_RM_FUNC_PARTITION_ALLOC = 1,
> > > > > > > > > +	IMX_SC_RM_FUNC_SET_CONFIDENTIAL = 31,
> > > > > > > > > +	IMX_SC_RM_FUNC_PARTITION_FREE = 2,
> > > > > > > > > +	IMX_SC_RM_FUNC_GET_DID = 26,
> > > > > > > > > +	IMX_SC_RM_FUNC_PARTITION_STATIC = 3,
> > > > > > > > > +	IMX_SC_RM_FUNC_PARTITION_LOCK = 4,
> > > > > > > > > +	IMX_SC_RM_FUNC_GET_PARTITION = 5,
> > > > > > > > > +	IMX_SC_RM_FUNC_SET_PARENT = 6,
> > > > > > > > > +	IMX_SC_RM_FUNC_MOVE_ALL = 7,
> > > > > > > > > +	IMX_SC_RM_FUNC_ASSIGN_RESOURCE = 8,
> > > > > > > > > +	IMX_SC_RM_FUNC_SET_RESOURCE_MOVABLE = 9,
> > > > > > > > > +	IMX_SC_RM_FUNC_SET_SUBSYS_RSRC_MOVABLE = 28,
> > > > > > > > > +	IMX_SC_RM_FUNC_SET_MASTER_ATTRIBUTES = 10,
> > > > > > > > > +	IMX_SC_RM_FUNC_SET_MASTER_SID = 11,
> > > > > > > > > +	IMX_SC_RM_FUNC_SET_PERIPHERAL_PERMISSIONS = 12,
> > > > > > > > > +	IMX_SC_RM_FUNC_IS_RESOURCE_OWNED = 13,
> > > > > > > > > +	IMX_SC_RM_FUNC_GET_RESOURCE_OWNER = 33,
> > > > > > > > > +	IMX_SC_RM_FUNC_IS_RESOURCE_MASTER = 14,
> > > > > > > > > +	IMX_SC_RM_FUNC_IS_RESOURCE_PERIPHERAL = 15,
> > > > > > > > > +	IMX_SC_RM_FUNC_GET_RESOURCE_INFO = 16,
> > > > > > > > > +	IMX_SC_RM_FUNC_MEMREG_ALLOC = 17,
> > > > > > > > > +	IMX_SC_RM_FUNC_MEMREG_SPLIT = 29,
> > > > > > > > > +	IMX_SC_RM_FUNC_MEMREG_FRAG = 32,
> > > > > > > > > +	IMX_SC_RM_FUNC_MEMREG_FREE = 18,
> > > > > > > > > +	IMX_SC_RM_FUNC_FIND_MEMREG = 30,
> > > > > > > > > +	IMX_SC_RM_FUNC_ASSIGN_MEMREG = 19,
> > > > > > > > > +	IMX_SC_RM_FUNC_SET_MEMREG_PERMISSIONS = 20,
> > > > > > > > > +	IMX_SC_RM_FUNC_IS_MEMREG_OWNED = 21,
> > > > > > > > > +	IMX_SC_RM_FUNC_GET_MEMREG_INFO = 22,
> > > > > > > > > +	IMX_SC_RM_FUNC_ASSIGN_PAD = 23,
> > > > > > > > > +	IMX_SC_RM_FUNC_SET_PAD_MOVABLE = 24,
> > > > > > > > > +	IMX_SC_RM_FUNC_IS_PAD_OWNED = 25,
> > > > > > > > > +	IMX_SC_RM_FUNC_DUMP = 27, };
> > > > > > > > > +
> > > > > > > > > +#if IS_ENABLED(CONFIG_IMX_SCU) bool
> > > > > > > > > +imx_sc_rm_is_resource_owned(struct imx_sc_ipc *ipc, u16
> > > > > > > > > +resource); #else static inline bool
> > > > > > > > > +imx_sc_rm_is_resource_owned(struct imx_sc_ipc *ipc, u16
> > > > resource) {
> > > > > > > > > +	return true;
> > > > > > > > > +}
> > > > > > > > > +#endif
> > > > > > > > > +#endif
> > > > > > > > > --
> > > > > > > > > 2.16.4

_______________________________________________
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 v4 1/4] KEYS: trusted: Add generic trusted keys framework
From: Jarkko Sakkinen @ 2020-06-02  7:14 UTC (permalink / raw)
  To: Sumit Garg
  Cc: tee-dev @ lists . linaro . org, Daniel Thompson, op-tee,
	Jonathan Corbet, James Bottomley, Janne Karhunen,
	Linux Doc Mailing List, James Morris, Mimi Zohar,
	Linux Kernel Mailing List, dhowells, linux-security-module,
	open list:ASYMMETRIC KEYS, Markus Wamser, Casey Schaufler,
	linux-integrity, Jens Wiklander, linux-arm-kernel,
	Serge E. Hallyn
In-Reply-To: <CAFA6WYP55W2xKtjHWWwu6Pbqy2TGY=eymwAoXxQh-5mF8deR6A@mail.gmail.com>

On Mon, Jun 01, 2020 at 02:41:55PM +0530, Sumit Garg wrote:
> > This, I think is wrong. You should have a compile time flag for TPM e.g.
> > CONFIG_TRUSTED_TPM, not this dynamic mess.
> >
> 
> The whole idea to have it dynamic was to have a common trusted keys
> module which could support both TPM and TEE implementation depending
> on hardware. I guess it may be useful in scenarios where a particular
> hardware supports a TPM chip while other doesn't but both need to run
> a common kernel image.

For now it should only scale to what is needed. No problems refining
it later when there is something to enable.

/Jarkko

_______________________________________________
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 2/4] firmware: imx: add resource management api
From: Aisheng Dong @ 2020-06-02  7:12 UTC (permalink / raw)
  To: Peng Fan, shawnguo@kernel.org, s.hauer@pengutronix.de
  Cc: festevam@gmail.com, Joakim Zhang, linux@rempel-privat.de,
	linux-arm-kernel@lists.infradead.org, kernel@pengutronix.de,
	Leonard Crestez, Daniel Baluta, linux-kernel@vger.kernel.org,
	dl-linux-imx
In-Reply-To: <DB6PR0402MB2760C2AC0CAEF7D8EEB1CA49888B0@DB6PR0402MB2760.eurprd04.prod.outlook.com>

> From: Peng Fan <peng.fan@nxp.com>
> Sent: Tuesday, June 2, 2020 1:24 PM
> 
> > Subject: RE: [PATCH 2/4] firmware: imx: add resource management api
> >
> > > From: Peng Fan <peng.fan@nxp.com>
> > > Sent: Thursday, April 23, 2020 3:00 PM
> > >
> > > Add resource management API, when we have multiple partition running
> > > together, resources not owned to current partition should not be used.
> > >
> > > Reviewed-by: Leonard Crestez <leonard.crestez@nxp.com>
> > > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > > ---
> > >  drivers/firmware/imx/Makefile       |  2 +-
> > >  drivers/firmware/imx/rm.c           | 40 +++++++++++++++++++++
> > >  include/linux/firmware/imx/sci.h    |  1 +
> > >  include/linux/firmware/imx/svc/rm.h | 69
> > > +++++++++++++++++++++++++++++++++++++
> > >  4 files changed, 111 insertions(+), 1 deletion(-)  create mode
> > > 100644 drivers/firmware/imx/rm.c  create mode 100644
> > > include/linux/firmware/imx/svc/rm.h
> > >
> > > diff --git a/drivers/firmware/imx/Makefile
> > > b/drivers/firmware/imx/Makefile index 08bc9ddfbdfb..17ea3613e142
> > > 100644
> > > --- a/drivers/firmware/imx/Makefile
> > > +++ b/drivers/firmware/imx/Makefile
> > > @@ -1,4 +1,4 @@
> > >  # SPDX-License-Identifier: GPL-2.0
> > >  obj-$(CONFIG_IMX_DSP)		+= imx-dsp.o
> > > -obj-$(CONFIG_IMX_SCU)		+= imx-scu.o misc.o imx-scu-irq.o
> > > +obj-$(CONFIG_IMX_SCU)		+= imx-scu.o misc.o imx-scu-irq.o rm.o
> > >  obj-$(CONFIG_IMX_SCU_PD)	+= scu-pd.o
> > > diff --git a/drivers/firmware/imx/rm.c b/drivers/firmware/imx/rm.c
> > > new file mode 100644 index 000000000000..7b0334de5486
> > > --- /dev/null
> > > +++ b/drivers/firmware/imx/rm.c
> > > @@ -0,0 +1,40 @@
> > > +// SPDX-License-Identifier: GPL-2.0+
> > > +/*
> > > + * Copyright 2020 NXP
> > > + *
> > > + * File containing client-side RPC functions for the RM service.
> > > +These
> > > + * function are ported to clients that communicate to the SC.
> > > + */
> > > +
> > > +#include <linux/firmware/imx/svc/rm.h>
> > > +
> > > +struct imx_sc_msg_rm_rsrc_owned {
> > > +	struct imx_sc_rpc_msg hdr;
> > > +	u16 resource;
> > > +} __packed __aligned(4);
> > > +
> > > +/*
> > > + * This function check @resource is owned by current partition or
> > > +not
> > > + *
> > > + * @param[in]     ipc         IPC handle
> > > + * @param[in]     resource    resource the control is associated with
> > > + *
> > > + * @return Returns 0 for success and < 0 for errors.
> > > + */
> > > +bool imx_sc_rm_is_resource_owned(struct imx_sc_ipc *ipc, u16
> > > +resource) {
> > > +	struct imx_sc_msg_rm_rsrc_owned msg;
> > > +	struct imx_sc_rpc_msg *hdr = &msg.hdr;
> > > +
> > > +	hdr->ver = IMX_SC_RPC_VERSION;
> > > +	hdr->svc = IMX_SC_RPC_SVC_RM;
> > > +	hdr->func = IMX_SC_RM_FUNC_IS_RESOURCE_OWNED;
> > > +	hdr->size = 2;
> > > +
> > > +	msg.resource = resource;
> > > +
> > > +	imx_scu_call_rpc(ipc, &msg, true);
> >
> > No need check err return?
> 
> No. it use hdr->func as the resource owner bool.
> However imx_scu_call_rpc also use hdr->func to check error value or not.
> 
> When hdr->func is 1, imx_sc_to_linux_errno will report it -EINVAL. However
> here 1 means not owned.

For this special case, pls add a code comment about it.
Refer to:
drivers/input/keyboard/imx_sc_pwrkey.c

Regards
Aisheng

> 
> >
> > > +
> > > +	return hdr->func;
> > > +}
> > > +EXPORT_SYMBOL(imx_sc_rm_is_resource_owned);
> > > diff --git a/include/linux/firmware/imx/sci.h
> > > b/include/linux/firmware/imx/sci.h
> > > index 17ba4e405129..b5c5a56f29be 100644
> > > --- a/include/linux/firmware/imx/sci.h
> > > +++ b/include/linux/firmware/imx/sci.h
> > > @@ -15,6 +15,7 @@
> > >
> > >  #include <linux/firmware/imx/svc/misc.h>  #include
> > > <linux/firmware/imx/svc/pm.h>
> > > +#include <linux/firmware/imx/svc/rm.h>
> > >
> > >  int imx_scu_enable_general_irq_channel(struct device *dev);  int
> > > imx_scu_irq_register_notifier(struct notifier_block *nb); diff --git
> > > a/include/linux/firmware/imx/svc/rm.h
> > > b/include/linux/firmware/imx/svc/rm.h
> > > new file mode 100644
> > > index 000000000000..fc6ea62d9d0e
> > > --- /dev/null
> > > +++ b/include/linux/firmware/imx/svc/rm.h
> > > @@ -0,0 +1,69 @@
> > > +/* SPDX-License-Identifier: GPL-2.0+ */
> > > +/*
> > > + * Copyright (C) 2016 Freescale Semiconductor, Inc.
> > > + * Copyright 2017-2019 NXP
> >
> > Update copyright
> 
> ok
> 
> >
> > > + *
> > > + * Header file containing the public API for the System Controller
> > > +(SC)
> > > + * Power Management (PM) function. This includes functions for
> > > +power state
> > > + * control, clock control, reset control, and wake-up event control.
> >
> > Fix the code comments
> >
> > Otherwise, I'm fine with this patch.
> Ok.
> 
> Thanks,
> Peng.
> 
> >
> > Regards
> > Aisheng
> >
> > > + *
> > > + * RM_SVC (SVC) Resource Management Service
> > > + *
> > > + * Module for the Resource Management (RM) service.
> > > + */
> > > +
> > > +#ifndef _SC_RM_API_H
> > > +#define _SC_RM_API_H
> > > +
> > > +#include <linux/firmware/imx/sci.h>
> > > +
> > > +/*
> > > + * This type is used to indicate RPC RM function calls.
> > > + */
> > > +enum imx_sc_rm_func {
> > > +	IMX_SC_RM_FUNC_UNKNOWN = 0,
> > > +	IMX_SC_RM_FUNC_PARTITION_ALLOC = 1,
> > > +	IMX_SC_RM_FUNC_SET_CONFIDENTIAL = 31,
> > > +	IMX_SC_RM_FUNC_PARTITION_FREE = 2,
> > > +	IMX_SC_RM_FUNC_GET_DID = 26,
> > > +	IMX_SC_RM_FUNC_PARTITION_STATIC = 3,
> > > +	IMX_SC_RM_FUNC_PARTITION_LOCK = 4,
> > > +	IMX_SC_RM_FUNC_GET_PARTITION = 5,
> > > +	IMX_SC_RM_FUNC_SET_PARENT = 6,
> > > +	IMX_SC_RM_FUNC_MOVE_ALL = 7,
> > > +	IMX_SC_RM_FUNC_ASSIGN_RESOURCE = 8,
> > > +	IMX_SC_RM_FUNC_SET_RESOURCE_MOVABLE = 9,
> > > +	IMX_SC_RM_FUNC_SET_SUBSYS_RSRC_MOVABLE = 28,
> > > +	IMX_SC_RM_FUNC_SET_MASTER_ATTRIBUTES = 10,
> > > +	IMX_SC_RM_FUNC_SET_MASTER_SID = 11,
> > > +	IMX_SC_RM_FUNC_SET_PERIPHERAL_PERMISSIONS = 12,
> > > +	IMX_SC_RM_FUNC_IS_RESOURCE_OWNED = 13,
> > > +	IMX_SC_RM_FUNC_GET_RESOURCE_OWNER = 33,
> > > +	IMX_SC_RM_FUNC_IS_RESOURCE_MASTER = 14,
> > > +	IMX_SC_RM_FUNC_IS_RESOURCE_PERIPHERAL = 15,
> > > +	IMX_SC_RM_FUNC_GET_RESOURCE_INFO = 16,
> > > +	IMX_SC_RM_FUNC_MEMREG_ALLOC = 17,
> > > +	IMX_SC_RM_FUNC_MEMREG_SPLIT = 29,
> > > +	IMX_SC_RM_FUNC_MEMREG_FRAG = 32,
> > > +	IMX_SC_RM_FUNC_MEMREG_FREE = 18,
> > > +	IMX_SC_RM_FUNC_FIND_MEMREG = 30,
> > > +	IMX_SC_RM_FUNC_ASSIGN_MEMREG = 19,
> > > +	IMX_SC_RM_FUNC_SET_MEMREG_PERMISSIONS = 20,
> > > +	IMX_SC_RM_FUNC_IS_MEMREG_OWNED = 21,
> > > +	IMX_SC_RM_FUNC_GET_MEMREG_INFO = 22,
> > > +	IMX_SC_RM_FUNC_ASSIGN_PAD = 23,
> > > +	IMX_SC_RM_FUNC_SET_PAD_MOVABLE = 24,
> > > +	IMX_SC_RM_FUNC_IS_PAD_OWNED = 25,
> > > +	IMX_SC_RM_FUNC_DUMP = 27,
> > > +};
> > > +
> > > +#if IS_ENABLED(CONFIG_IMX_SCU)
> > > +bool imx_sc_rm_is_resource_owned(struct imx_sc_ipc *ipc, u16
> > > +resource); #else static inline bool
> > > +imx_sc_rm_is_resource_owned(struct imx_sc_ipc *ipc, u16 resource) {
> > > +	return true;
> > > +}
> > > +#endif
> > > +#endif
> > > --
> > > 2.16.4

_______________________________________________
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 v4 1/4] KEYS: trusted: Add generic trusted keys framework
From: Jarkko Sakkinen @ 2020-06-02  7:08 UTC (permalink / raw)
  To: Sumit Garg
  Cc: tee-dev @ lists . linaro . org, Daniel Thompson, op-tee,
	Jonathan Corbet, James Bottomley, Janne Karhunen,
	Linux Doc Mailing List, James Morris, Mimi Zohar,
	Linux Kernel Mailing List, dhowells, linux-security-module,
	open list:ASYMMETRIC KEYS, Markus Wamser, Casey Schaufler,
	linux-integrity, Jens Wiklander, linux-arm-kernel,
	Serge E. Hallyn
In-Reply-To: <CAFA6WYMN01WSsMhvqmtzG+hmnQ9+_MfobaH9c_LZ1wj9Z_xDjw@mail.gmail.com>

On Mon, Jun 01, 2020 at 02:20:26PM +0530, Sumit Garg wrote:
> On Mon, 1 Jun 2020 at 07:30, Jarkko Sakkinen
> <jarkko.sakkinen@linux.intel.com> wrote:
> >
> > On Wed, May 06, 2020 at 03:10:14PM +0530, Sumit Garg wrote:
> > > Current trusted keys framework is tightly coupled to use TPM device as
> > > an underlying implementation which makes it difficult for implementations
> > > like Trusted Execution Environment (TEE) etc. to provide trusked keys
> > > support in case platform doesn't posses a TPM device.
> > >
> > > So this patch tries to add generic trusted keys framework where underlying
> > > implemtations like TPM, TEE etc. could be easily plugged-in.
> > >
> > > Suggested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> > > Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
> > > ---
> > >  include/keys/trusted-type.h                 |  45 ++++
> > >  include/keys/trusted_tpm.h                  |  15 --
> > >  security/keys/trusted-keys/Makefile         |   1 +
> > >  security/keys/trusted-keys/trusted_common.c | 333 +++++++++++++++++++++++++++
> >
> > I think trusted_core.c would be a better name (less ambiguous).
> >
> 
> Okay.
> 
> > >  security/keys/trusted-keys/trusted_tpm1.c   | 335 +++++-----------------------
> > >  5 files changed, 437 insertions(+), 292 deletions(-)
> > >  create mode 100644 security/keys/trusted-keys/trusted_common.c
> > >
> > > diff --git a/include/keys/trusted-type.h b/include/keys/trusted-type.h
> > > index a94c03a..5559010 100644
> > > --- a/include/keys/trusted-type.h
> > > +++ b/include/keys/trusted-type.h
> > > @@ -40,6 +40,51 @@ struct trusted_key_options {
> > >       uint32_t policyhandle;
> > >  };
> > >
> > > +struct trusted_key_ops {
> > > +     /*
> > > +      * flag to indicate if trusted key implementation supports migration
> > > +      * or not.
> > > +      */
> > > +     unsigned char migratable;
> > > +
> > > +     /* trusted key init */
> > > +     int (*init)(void);
> >
> > /* Init a key. */
> >
> 
> This API isn't initializing a key but rather the underlying interface
> (see init_tpm_trusted()). So how about:
> 
> /* Initialize key interface */

Sure (also for others can use common sense).

> 
> > > +
> > > +     /* seal a trusted key */
> > > +     int (*seal)(struct trusted_key_payload *p, char *datablob);
> >
> > /* Seal a key. */
> >
> 
> Ack.
> 
> > > +
> > > +     /* unseal a trusted key */
> > > +     int (*unseal)(struct trusted_key_payload *p, char *datablob);
> >
> > /* Unseal a key. */
> >
> 
> Ack.
> 
> > > +
> > > +     /* get random trusted key */
> > > +     int (*get_random)(unsigned char *key, size_t key_len);
> >
> > /* Get a randomized key. */
> >
> 
> Ack.
> 
> > > +
> > > +     /* trusted key cleanup */
> > > +     void (*cleanup)(void);
> >
> > Please remove this from this commit since it is not in use in the scope
> > of this commit. You should instead make a separate commit just for this
> > callback, which explains what it is and how it will be used in the
> > follow up commits.
> >
> 
> This API is pretty much relevant to TPM as well (see:
> cleanup_tpm_trusted()) but I guess "cleanup()" terminology is bringing
> up some confusion, so how about to call it "exit()" instead?
> 
> >
> > > +};
> > > +
> > >  extern struct key_type key_type_trusted;
> > > +#if defined(CONFIG_TCG_TPM)
> > > +extern struct trusted_key_ops tpm_trusted_key_ops;
> > > +#endif
> > > +
> > > +#define TRUSTED_DEBUG 0
> > > +
> > > +#if TRUSTED_DEBUG
> > > +static inline void dump_payload(struct trusted_key_payload *p)
> > > +{
> > > +     pr_info("trusted_key: key_len %d\n", p->key_len);
> > > +     print_hex_dump(KERN_INFO, "key ", DUMP_PREFIX_NONE,
> > > +                    16, 1, p->key, p->key_len, 0);
> > > +     pr_info("trusted_key: bloblen %d\n", p->blob_len);
> > > +     print_hex_dump(KERN_INFO, "blob ", DUMP_PREFIX_NONE,
> > > +                    16, 1, p->blob, p->blob_len, 0);
> > > +     pr_info("trusted_key: migratable %d\n", p->migratable);
> > > +}
> > > +#else
> > > +static inline void dump_payload(struct trusted_key_payload *p)
> > > +{
> > > +}
> > > +#endif
> > >
> > >  #endif /* _KEYS_TRUSTED_TYPE_H */
> > > diff --git a/include/keys/trusted_tpm.h b/include/keys/trusted_tpm.h
> > > index a56d8e1..5753231 100644
> > > --- a/include/keys/trusted_tpm.h
> > > +++ b/include/keys/trusted_tpm.h
> > > @@ -60,17 +60,6 @@ static inline void dump_options(struct trusted_key_options *o)
> > >                      16, 1, o->pcrinfo, o->pcrinfo_len, 0);
> > >  }
> > >
> > > -static inline void dump_payload(struct trusted_key_payload *p)
> > > -{
> > > -     pr_info("trusted_key: key_len %d\n", p->key_len);
> > > -     print_hex_dump(KERN_INFO, "key ", DUMP_PREFIX_NONE,
> > > -                    16, 1, p->key, p->key_len, 0);
> > > -     pr_info("trusted_key: bloblen %d\n", p->blob_len);
> > > -     print_hex_dump(KERN_INFO, "blob ", DUMP_PREFIX_NONE,
> > > -                    16, 1, p->blob, p->blob_len, 0);
> > > -     pr_info("trusted_key: migratable %d\n", p->migratable);
> > > -}
> > > -
> > >  static inline void dump_sess(struct osapsess *s)
> > >  {
> > >       print_hex_dump(KERN_INFO, "trusted-key: handle ", DUMP_PREFIX_NONE,
> > > @@ -96,10 +85,6 @@ static inline void dump_options(struct trusted_key_options *o)
> > >  {
> > >  }
> > >
> > > -static inline void dump_payload(struct trusted_key_payload *p)
> > > -{
> > > -}
> > > -
> > >  static inline void dump_sess(struct osapsess *s)
> > >  {
> > >  }
> > > diff --git a/security/keys/trusted-keys/Makefile b/security/keys/trusted-keys/Makefile
> > > index 7b73ceb..2b1085b 100644
> > > --- a/security/keys/trusted-keys/Makefile
> > > +++ b/security/keys/trusted-keys/Makefile
> > > @@ -4,5 +4,6 @@
> > >  #
> > >
> > >  obj-$(CONFIG_TRUSTED_KEYS) += trusted.o
> > > +trusted-y += trusted_common.o
> > >  trusted-y += trusted_tpm1.o
> > >  trusted-y += trusted_tpm2.o
> > > diff --git a/security/keys/trusted-keys/trusted_common.c b/security/keys/trusted-keys/trusted_common.c
> > > new file mode 100644
> > > index 0000000..9bfd081
> > > --- /dev/null
> > > +++ b/security/keys/trusted-keys/trusted_common.c
> > > @@ -0,0 +1,333 @@
> > > +// SPDX-License-Identifier: GPL-2.0-only
> > > +/*
> > > + * Copyright (C) 2010 IBM Corporation
> > > + * Copyright (c) 2019, Linaro Limited
> > > + *
> > > + * Author:
> > > + * David Safford <safford@us.ibm.com>
> > > + * Added generic trusted key framework: Sumit Garg <sumit.garg@linaro.org>
> > > + *
> > > + * See Documentation/security/keys/trusted-encrypted.rst
> > > + */
> > > +
> > > +#include <keys/user-type.h>
> > > +#include <keys/trusted-type.h>
> > > +#include <linux/capability.h>
> > > +#include <linux/err.h>
> > > +#include <linux/init.h>
> > > +#include <linux/key-type.h>
> > > +#include <linux/module.h>
> > > +#include <linux/parser.h>
> > > +#include <linux/rcupdate.h>
> > > +#include <linux/slab.h>
> > > +#include <linux/string.h>
> > > +#include <linux/uaccess.h>
> > > +
> > > +static struct trusted_key_ops *available_tk_ops[] = {
> > > +#if defined(CONFIG_TCG_TPM)
> > > +     &tpm_trusted_key_ops,
> > > +#endif
> > > +};
> > > +static struct trusted_key_ops *tk_ops;
> > > +
> > > +enum {
> > > +     Opt_err,
> > > +     Opt_new, Opt_load, Opt_update,
> > > +};
> > > +
> > > +static const match_table_t key_tokens = {
> > > +     {Opt_new, "new"},
> > > +     {Opt_load, "load"},
> > > +     {Opt_update, "update"},
> > > +     {Opt_err, NULL}
> > > +};
> > > +
> > > +/*
> > > + * datablob_parse - parse the keyctl data and fill in the
> > > + *                  payload structure
> > > + *
> > > + * On success returns 0, otherwise -EINVAL.
> > > + */
> > > +static int datablob_parse(char *datablob, struct trusted_key_payload *p)
> > > +{
> > > +     substring_t args[MAX_OPT_ARGS];
> > > +     long keylen;
> > > +     int ret = -EINVAL;
> > > +     int key_cmd;
> > > +     char *c;
> > > +
> > > +     /* main command */
> > > +     c = strsep(&datablob, " \t");
> > > +     if (!c)
> > > +             return -EINVAL;
> > > +     key_cmd = match_token(c, key_tokens, args);
> > > +     switch (key_cmd) {
> > > +     case Opt_new:
> > > +             /* first argument is key size */
> > > +             c = strsep(&datablob, " \t");
> > > +             if (!c)
> > > +                     return -EINVAL;
> > > +             ret = kstrtol(c, 10, &keylen);
> > > +             if (ret < 0 || keylen < MIN_KEY_SIZE || keylen > MAX_KEY_SIZE)
> > > +                     return -EINVAL;
> > > +             p->key_len = keylen;
> > > +             ret = Opt_new;
> > > +             break;
> > > +     case Opt_load:
> > > +             /* first argument is sealed blob */
> > > +             c = strsep(&datablob, " \t");
> > > +             if (!c)
> > > +                     return -EINVAL;
> > > +             p->blob_len = strlen(c) / 2;
> > > +             if (p->blob_len > MAX_BLOB_SIZE)
> > > +                     return -EINVAL;
> > > +             ret = hex2bin(p->blob, c, p->blob_len);
> > > +             if (ret < 0)
> > > +                     return -EINVAL;
> > > +             ret = Opt_load;
> > > +             break;
> > > +     case Opt_update:
> > > +             ret = Opt_update;
> > > +             break;
> > > +     case Opt_err:
> > > +             return -EINVAL;
> > > +     }
> > > +     return ret;
> > > +}
> > > +
> > > +static struct trusted_key_payload *trusted_payload_alloc(struct key *key)
> > > +{
> > > +     struct trusted_key_payload *p = NULL;
> > > +     int ret;
> > > +
> > > +     ret = key_payload_reserve(key, sizeof(*p));
> > > +     if (ret < 0)
> > > +             return p;
> > > +     p = kzalloc(sizeof(*p), GFP_KERNEL);
> > > +
> > > +     p->migratable = tk_ops->migratable;
> > > +
> > > +     return p;
> > > +}
> > > +
> > > +/*
> > > + * trusted_instantiate - create a new trusted key
> > > + *
> > > + * Unseal an existing trusted blob or, for a new key, get a
> > > + * random key, then seal and create a trusted key-type key,
> > > + * adding it to the specified keyring.
> > > + *
> > > + * On success, return 0. Otherwise return errno.
> > > + */
> > > +static int trusted_instantiate(struct key *key,
> > > +                            struct key_preparsed_payload *prep)
> > > +{
> > > +     struct trusted_key_payload *payload = NULL;
> > > +     size_t datalen = prep->datalen;
> > > +     char *datablob;
> > > +     int ret = 0;
> > > +     int key_cmd;
> > > +     size_t key_len;
> > > +
> > > +     if (datalen <= 0 || datalen > 32767 || !prep->data)
> > > +             return -EINVAL;
> > > +
> > > +     datablob = kmalloc(datalen + 1, GFP_KERNEL);
> > > +     if (!datablob)
> > > +             return -ENOMEM;
> > > +     memcpy(datablob, prep->data, datalen);
> > > +     datablob[datalen] = '\0';
> > > +
> > > +     payload = trusted_payload_alloc(key);
> > > +     if (!payload) {
> > > +             ret = -ENOMEM;
> > > +             goto out;
> > > +     }
> > > +
> > > +     key_cmd = datablob_parse(datablob, payload);
> > > +     if (key_cmd < 0) {
> > > +             ret = key_cmd;
> > > +             goto out;
> > > +     }
> > > +
> > > +     dump_payload(payload);
> > > +
> > > +     switch (key_cmd) {
> > > +     case Opt_load:
> > > +             ret = tk_ops->unseal(payload, datablob);
> > > +             dump_payload(payload);
> > > +             if (ret < 0)
> > > +                     pr_info("trusted_key: key_unseal failed (%d)\n", ret);
> > > +             break;
> > > +     case Opt_new:
> > > +             key_len = payload->key_len;
> > > +             ret = tk_ops->get_random(payload->key, key_len);
> > > +             if (ret != key_len) {
> > > +                     pr_info("trusted_key: key_create failed (%d)\n", ret);
> > > +                     goto out;
> > > +             }
> > > +
> > > +             ret = tk_ops->seal(payload, datablob);
> > > +             if (ret < 0)
> > > +                     pr_info("trusted_key: key_seal failed (%d)\n", ret);
> > > +             break;
> > > +     default:
> > > +             ret = -EINVAL;
> > > +     }
> > > +out:
> > > +     kzfree(datablob);
> > > +     if (!ret)
> > > +             rcu_assign_keypointer(key, payload);
> > > +     else
> > > +             kzfree(payload);
> > > +     return ret;
> > > +}
> > > +
> > > +static void trusted_rcu_free(struct rcu_head *rcu)
> > > +{
> > > +     struct trusted_key_payload *p;
> > > +
> > > +     p = container_of(rcu, struct trusted_key_payload, rcu);
> > > +     kzfree(p);
> > > +}
> > > +
> > > +/*
> > > + * trusted_update - reseal an existing key with new PCR values
> > > + */
> > > +static int trusted_update(struct key *key, struct key_preparsed_payload *prep)
> > > +{
> > > +     struct trusted_key_payload *p;
> > > +     struct trusted_key_payload *new_p;
> > > +     size_t datalen = prep->datalen;
> > > +     char *datablob;
> > > +     int ret = 0;
> > > +
> > > +     if (key_is_negative(key))
> > > +             return -ENOKEY;
> > > +     p = key->payload.data[0];
> > > +     if (!p->migratable)
> > > +             return -EPERM;
> > > +     if (datalen <= 0 || datalen > 32767 || !prep->data)
> > > +             return -EINVAL;
> > > +
> > > +     datablob = kmalloc(datalen + 1, GFP_KERNEL);
> > > +     if (!datablob)
> > > +             return -ENOMEM;
> > > +
> > > +     new_p = trusted_payload_alloc(key);
> > > +     if (!new_p) {
> > > +             ret = -ENOMEM;
> > > +             goto out;
> > > +     }
> > > +
> > > +     memcpy(datablob, prep->data, datalen);
> > > +     datablob[datalen] = '\0';
> > > +     ret = datablob_parse(datablob, new_p);
> > > +     if (ret != Opt_update) {
> > > +             ret = -EINVAL;
> > > +             kzfree(new_p);
> > > +             goto out;
> > > +     }
> > > +
> > > +     /* copy old key values, and reseal with new pcrs */
> > > +     new_p->migratable = p->migratable;
> > > +     new_p->key_len = p->key_len;
> > > +     memcpy(new_p->key, p->key, p->key_len);
> > > +     dump_payload(p);
> > > +     dump_payload(new_p);
> > > +
> > > +     ret = tk_ops->seal(new_p, datablob);
> > > +     if (ret < 0) {
> > > +             pr_info("trusted_key: key_seal failed (%d)\n", ret);
> > > +             kzfree(new_p);
> > > +             goto out;
> > > +     }
> > > +
> > > +     rcu_assign_keypointer(key, new_p);
> > > +     call_rcu(&p->rcu, trusted_rcu_free);
> > > +out:
> > > +     kzfree(datablob);
> > > +     return ret;
> > > +}
> > > +
> > > +/*
> > > + * trusted_read - copy the sealed blob data to userspace in hex.
> > > + * On success, return to userspace the trusted key datablob size.
> > > + */
> > > +static long trusted_read(const struct key *key, char *buffer,
> > > +                      size_t buflen)
> > > +{
> > > +     const struct trusted_key_payload *p;
> > > +     char *bufp;
> > > +     int i;
> > > +
> > > +     p = dereference_key_locked(key);
> > > +     if (!p)
> > > +             return -EINVAL;
> > > +
> > > +     if (buffer && buflen >= 2 * p->blob_len) {
> > > +             bufp = buffer;
> > > +             for (i = 0; i < p->blob_len; i++)
> > > +                     bufp = hex_byte_pack(bufp, p->blob[i]);
> > > +     }
> > > +     return 2 * p->blob_len;
> > > +}
> > > +
> > > +/*
> > > + * trusted_destroy - clear and free the key's payload
> > > + */
> > > +static void trusted_destroy(struct key *key)
> > > +{
> > > +     kzfree(key->payload.data[0]);
> > > +}
> > > +
> > > +struct key_type key_type_trusted = {
> > > +     .name = "trusted",
> > > +     .instantiate = trusted_instantiate,
> > > +     .update = trusted_update,
> > > +     .destroy = trusted_destroy,
> > > +     .describe = user_describe,
> > > +     .read = trusted_read,
> > > +};
> > > +EXPORT_SYMBOL_GPL(key_type_trusted);
> > > +
> > > +static int __init init_trusted(void)
> > > +{
> > > +     int i, ret = 0;
> > > +
> > > +     for (i = 0; i < sizeof(available_tk_ops); i++) {
> > > +             tk_ops = available_tk_ops[i];
> > > +
> > > +             if (!(tk_ops && tk_ops->init && tk_ops->seal &&
> > > +                   tk_ops->unseal && tk_ops->get_random))
> > > +                     continue;
> > > +
> > > +             ret = tk_ops->init();
> > > +             if (ret) {
> > > +                     if (tk_ops->cleanup)
> > > +                             tk_ops->cleanup();
> > > +             } else {
> > > +                     break;
> > > +             }
> > > +     }
> > > +
> > > +     /*
> > > +      * encrypted_keys.ko depends on successful load of this module even if
> > > +      * trusted key implementation is not found.
> > > +      */
> > > +     if (ret == -ENODEV)
> > > +             return 0;
> > > +
> > > +     return ret;
> > > +}
> > > +
> > > +static void __exit cleanup_trusted(void)
> > > +{
> > > +     if (tk_ops->cleanup)
> > > +             tk_ops->cleanup();
> > > +}
> > > +
> > > +late_initcall(init_trusted);
> > > +module_exit(cleanup_trusted);
> > > +
> > > +MODULE_LICENSE("GPL");
> > > diff --git a/security/keys/trusted-keys/trusted_tpm1.c b/security/keys/trusted-keys/trusted_tpm1.c
> > > index 8001ab0..32fd1ea 100644
> > > --- a/security/keys/trusted-keys/trusted_tpm1.c
> > > +++ b/security/keys/trusted-keys/trusted_tpm1.c
> > > @@ -1,29 +1,26 @@
> > >  // SPDX-License-Identifier: GPL-2.0-only
> > >  /*
> > >   * Copyright (C) 2010 IBM Corporation
> > > + * Copyright (c) 2019, Linaro Limited
> > >   *
> > >   * Author:
> > >   * David Safford <safford@us.ibm.com>
> > > + * Switch to generic trusted key framework: Sumit Garg <sumit.garg@linaro.org>
> > >   *
> > >   * See Documentation/security/keys/trusted-encrypted.rst
> > >   */
> > >
> > >  #include <crypto/hash_info.h>
> > > -#include <linux/uaccess.h>
> > > -#include <linux/module.h>
> > >  #include <linux/init.h>
> > >  #include <linux/slab.h>
> > >  #include <linux/parser.h>
> > >  #include <linux/string.h>
> > >  #include <linux/err.h>
> > > -#include <keys/user-type.h>
> > >  #include <keys/trusted-type.h>
> > >  #include <linux/key-type.h>
> > > -#include <linux/rcupdate.h>
> > >  #include <linux/crypto.h>
> > >  #include <crypto/hash.h>
> > >  #include <crypto/sha.h>
> > > -#include <linux/capability.h>
> > >  #include <linux/tpm.h>
> > >  #include <linux/tpm_command.h>
> > >
> > > @@ -703,7 +700,6 @@ static int key_unseal(struct trusted_key_payload *p,
> > >
> > >  enum {
> > >       Opt_err,
> > > -     Opt_new, Opt_load, Opt_update,
> > >       Opt_keyhandle, Opt_keyauth, Opt_blobauth,
> > >       Opt_pcrinfo, Opt_pcrlock, Opt_migratable,
> > >       Opt_hash,
> > > @@ -712,9 +708,6 @@ enum {
> > >  };
> > >
> > >  static const match_table_t key_tokens = {
> > > -     {Opt_new, "new"},
> > > -     {Opt_load, "load"},
> > > -     {Opt_update, "update"},
> > >       {Opt_keyhandle, "keyhandle=%s"},
> > >       {Opt_keyauth, "keyauth=%s"},
> > >       {Opt_blobauth, "blobauth=%s"},
> > > @@ -841,71 +834,6 @@ static int getoptions(char *c, struct trusted_key_payload *pay,
> > >       return 0;
> > >  }
> > >
> > > -/*
> > > - * datablob_parse - parse the keyctl data and fill in the
> > > - *               payload and options structures
> > > - *
> > > - * On success returns 0, otherwise -EINVAL.
> > > - */
> > > -static int datablob_parse(char *datablob, struct trusted_key_payload *p,
> > > -                       struct trusted_key_options *o)
> > > -{
> > > -     substring_t args[MAX_OPT_ARGS];
> > > -     long keylen;
> > > -     int ret = -EINVAL;
> > > -     int key_cmd;
> > > -     char *c;
> > > -
> > > -     /* main command */
> > > -     c = strsep(&datablob, " \t");
> > > -     if (!c)
> > > -             return -EINVAL;
> > > -     key_cmd = match_token(c, key_tokens, args);
> > > -     switch (key_cmd) {
> > > -     case Opt_new:
> > > -             /* first argument is key size */
> > > -             c = strsep(&datablob, " \t");
> > > -             if (!c)
> > > -                     return -EINVAL;
> > > -             ret = kstrtol(c, 10, &keylen);
> > > -             if (ret < 0 || keylen < MIN_KEY_SIZE || keylen > MAX_KEY_SIZE)
> > > -                     return -EINVAL;
> > > -             p->key_len = keylen;
> > > -             ret = getoptions(datablob, p, o);
> > > -             if (ret < 0)
> > > -                     return ret;
> > > -             ret = Opt_new;
> > > -             break;
> > > -     case Opt_load:
> > > -             /* first argument is sealed blob */
> > > -             c = strsep(&datablob, " \t");
> > > -             if (!c)
> > > -                     return -EINVAL;
> > > -             p->blob_len = strlen(c) / 2;
> > > -             if (p->blob_len > MAX_BLOB_SIZE)
> > > -                     return -EINVAL;
> > > -             ret = hex2bin(p->blob, c, p->blob_len);
> > > -             if (ret < 0)
> > > -                     return -EINVAL;
> > > -             ret = getoptions(datablob, p, o);
> > > -             if (ret < 0)
> > > -                     return ret;
> > > -             ret = Opt_load;
> > > -             break;
> > > -     case Opt_update:
> > > -             /* all arguments are options */
> > > -             ret = getoptions(datablob, p, o);
> > > -             if (ret < 0)
> > > -                     return ret;
> > > -             ret = Opt_update;
> > > -             break;
> > > -     case Opt_err:
> > > -             return -EINVAL;
> > > -             break;
> > > -     }
> > > -     return ret;
> > > -}
> > > -
> > >  static struct trusted_key_options *trusted_options_alloc(void)
> > >  {
> > >       struct trusted_key_options *options;
> > > @@ -926,248 +854,99 @@ static struct trusted_key_options *trusted_options_alloc(void)
> > >       return options;
> > >  }
> > >
> > > -static struct trusted_key_payload *trusted_payload_alloc(struct key *key)
> > > +static int tpm_tk_seal(struct trusted_key_payload *p, char *datablob)
> > >  {
> > > -     struct trusted_key_payload *p = NULL;
> > > -     int ret;
> > > -
> > > -     ret = key_payload_reserve(key, sizeof *p);
> > > -     if (ret < 0)
> > > -             return p;
> > > -     p = kzalloc(sizeof *p, GFP_KERNEL);
> > > -     if (p)
> > > -             p->migratable = 1; /* migratable by default */
> > > -     return p;
> > > -}
> > > -
> > > -/*
> > > - * trusted_instantiate - create a new trusted key
> > > - *
> > > - * Unseal an existing trusted blob or, for a new key, get a
> > > - * random key, then seal and create a trusted key-type key,
> > > - * adding it to the specified keyring.
> > > - *
> > > - * On success, return 0. Otherwise return errno.
> > > - */
> > > -static int trusted_instantiate(struct key *key,
> > > -                            struct key_preparsed_payload *prep)
> > > -{
> > > -     struct trusted_key_payload *payload = NULL;
> > >       struct trusted_key_options *options = NULL;
> > > -     size_t datalen = prep->datalen;
> > > -     char *datablob;
> > >       int ret = 0;
> > > -     int key_cmd;
> > > -     size_t key_len;
> > >       int tpm2;
> > >
> > >       tpm2 = tpm_is_tpm2(chip);
> > >       if (tpm2 < 0)
> > >               return tpm2;
> > >
> > > -     if (datalen <= 0 || datalen > 32767 || !prep->data)
> > > -             return -EINVAL;
> > > -
> > > -     datablob = kmalloc(datalen + 1, GFP_KERNEL);
> > > -     if (!datablob)
> > > -             return -ENOMEM;
> > > -     memcpy(datablob, prep->data, datalen);
> > > -     datablob[datalen] = '\0';
> > > -
> > >       options = trusted_options_alloc();
> > > -     if (!options) {
> > > -             ret = -ENOMEM;
> > > -             goto out;
> > > -     }
> > > -     payload = trusted_payload_alloc(key);
> > > -     if (!payload) {
> > > -             ret = -ENOMEM;
> > > -             goto out;
> > > -     }
> > > +     if (!options)
> > > +             return -ENOMEM;
> > >
> > > -     key_cmd = datablob_parse(datablob, payload, options);
> > > -     if (key_cmd < 0) {
> > > -             ret = key_cmd;
> > > +     ret = getoptions(datablob, p, options);
> > > +     if (ret < 0)
> > >               goto out;
> > > -     }
> > > +     dump_options(options);
> > >
> > >       if (!options->keyhandle) {
> > >               ret = -EINVAL;
> > >               goto out;
> > >       }
> > >
> > > -     dump_payload(payload);
> > > -     dump_options(options);
> > > +     if (tpm2)
> > > +             ret = tpm2_seal_trusted(chip, p, options);
> > > +     else
> > > +             ret = key_seal(p, options);
> > > +     if (ret < 0) {
> > > +             pr_info("tpm_trusted_key: key_seal failed (%d)\n", ret);
> > > +             goto out;
> > > +     }
> > >
> > > -     switch (key_cmd) {
> > > -     case Opt_load:
> > > -             if (tpm2)
> > > -                     ret = tpm2_unseal_trusted(chip, payload, options);
> > > -             else
> > > -                     ret = key_unseal(payload, options);
> > > -             dump_payload(payload);
> > > -             dump_options(options);
> > > -             if (ret < 0)
> > > -                     pr_info("trusted_key: key_unseal failed (%d)\n", ret);
> > > -             break;
> > > -     case Opt_new:
> > > -             key_len = payload->key_len;
> > > -             ret = tpm_get_random(chip, payload->key, key_len);
> > > -             if (ret != key_len) {
> > > -                     pr_info("trusted_key: key_create failed (%d)\n", ret);
> > > +     if (options->pcrlock) {
> > > +             ret = pcrlock(options->pcrlock);
> > > +             if (ret < 0) {
> > > +                     pr_info("tpm_trusted_key: pcrlock failed (%d)\n", ret);
> > >                       goto out;
> > >               }
> > > -             if (tpm2)
> > > -                     ret = tpm2_seal_trusted(chip, payload, options);
> > > -             else
> > > -                     ret = key_seal(payload, options);
> > > -             if (ret < 0)
> > > -                     pr_info("trusted_key: key_seal failed (%d)\n", ret);
> > > -             break;
> > > -     default:
> > > -             ret = -EINVAL;
> > > -             goto out;
> > >       }
> > > -     if (!ret && options->pcrlock)
> > > -             ret = pcrlock(options->pcrlock);
> > >  out:
> > > -     kzfree(datablob);
> > >       kzfree(options);
> > > -     if (!ret)
> > > -             rcu_assign_keypointer(key, payload);
> > > -     else
> > > -             kzfree(payload);
> > >       return ret;
> > >  }
> > >
> > > -static void trusted_rcu_free(struct rcu_head *rcu)
> > > +static int tpm_tk_unseal(struct trusted_key_payload *p, char *datablob)
> > >  {
> > > -     struct trusted_key_payload *p;
> > > -
> > > -     p = container_of(rcu, struct trusted_key_payload, rcu);
> > > -     kzfree(p);
> > > -}
> > > -
> > > -/*
> > > - * trusted_update - reseal an existing key with new PCR values
> > > - */
> > > -static int trusted_update(struct key *key, struct key_preparsed_payload *prep)
> > > -{
> > > -     struct trusted_key_payload *p;
> > > -     struct trusted_key_payload *new_p;
> > > -     struct trusted_key_options *new_o;
> > > -     size_t datalen = prep->datalen;
> > > -     char *datablob;
> > > +     struct trusted_key_options *options = NULL;
> > >       int ret = 0;
> > > +     int tpm2;
> > >
> > > -     if (key_is_negative(key))
> > > -             return -ENOKEY;
> > > -     p = key->payload.data[0];
> > > -     if (!p->migratable)
> > > -             return -EPERM;
> > > -     if (datalen <= 0 || datalen > 32767 || !prep->data)
> > > -             return -EINVAL;
> > > +     tpm2 = tpm_is_tpm2(chip);
> > > +     if (tpm2 < 0)
> > > +             return tpm2;
> > >
> > > -     datablob = kmalloc(datalen + 1, GFP_KERNEL);
> > > -     if (!datablob)
> > > +     options = trusted_options_alloc();
> > > +     if (!options)
> > >               return -ENOMEM;
> > > -     new_o = trusted_options_alloc();
> > > -     if (!new_o) {
> > > -             ret = -ENOMEM;
> > > -             goto out;
> > > -     }
> > > -     new_p = trusted_payload_alloc(key);
> > > -     if (!new_p) {
> > > -             ret = -ENOMEM;
> > > -             goto out;
> > > -     }
> > >
> > > -     memcpy(datablob, prep->data, datalen);
> > > -     datablob[datalen] = '\0';
> > > -     ret = datablob_parse(datablob, new_p, new_o);
> > > -     if (ret != Opt_update) {
> > > -             ret = -EINVAL;
> > > -             kzfree(new_p);
> > > +     ret = getoptions(datablob, p, options);
> > > +     if (ret < 0)
> > >               goto out;
> > > -     }
> > > +     dump_options(options);
> > >
> > > -     if (!new_o->keyhandle) {
> > > +     if (!options->keyhandle) {
> > >               ret = -EINVAL;
> > > -             kzfree(new_p);
> > >               goto out;
> > >       }
> > >
> > > -     /* copy old key values, and reseal with new pcrs */
> > > -     new_p->migratable = p->migratable;
> > > -     new_p->key_len = p->key_len;
> > > -     memcpy(new_p->key, p->key, p->key_len);
> > > -     dump_payload(p);
> > > -     dump_payload(new_p);
> > > +     if (tpm2)
> > > +             ret = tpm2_unseal_trusted(chip, p, options);
> > > +     else
> > > +             ret = key_unseal(p, options);
> > > +     if (ret < 0)
> > > +             pr_info("tpm_trusted_key: key_unseal failed (%d)\n", ret);
> > >
> > > -     ret = key_seal(new_p, new_o);
> > > -     if (ret < 0) {
> > > -             pr_info("trusted_key: key_seal failed (%d)\n", ret);
> > > -             kzfree(new_p);
> > > -             goto out;
> > > -     }
> > > -     if (new_o->pcrlock) {
> > > -             ret = pcrlock(new_o->pcrlock);
> > > +     if (options->pcrlock) {
> > > +             ret = pcrlock(options->pcrlock);
> > >               if (ret < 0) {
> > > -                     pr_info("trusted_key: pcrlock failed (%d)\n", ret);
> > > -                     kzfree(new_p);
> > > +                     pr_info("tpm_trusted_key: pcrlock failed (%d)\n", ret);
> > >                       goto out;
> > >               }
> > >       }
> > > -     rcu_assign_keypointer(key, new_p);
> > > -     call_rcu(&p->rcu, trusted_rcu_free);
> > >  out:
> > > -     kzfree(datablob);
> > > -     kzfree(new_o);
> > > +     kzfree(options);
> > >       return ret;
> > >  }
> > >
> > > -/*
> > > - * trusted_read - copy the sealed blob data to userspace in hex.
> > > - * On success, return to userspace the trusted key datablob size.
> > > - */
> > > -static long trusted_read(const struct key *key, char *buffer,
> > > -                      size_t buflen)
> > > -{
> > > -     const struct trusted_key_payload *p;
> > > -     char *bufp;
> > > -     int i;
> > > -
> > > -     p = dereference_key_locked(key);
> > > -     if (!p)
> > > -             return -EINVAL;
> > > -
> > > -     if (buffer && buflen >= 2 * p->blob_len) {
> > > -             bufp = buffer;
> > > -             for (i = 0; i < p->blob_len; i++)
> > > -                     bufp = hex_byte_pack(bufp, p->blob[i]);
> > > -     }
> > > -     return 2 * p->blob_len;
> > > -}
> > > -
> > > -/*
> > > - * trusted_destroy - clear and free the key's payload
> > > - */
> > > -static void trusted_destroy(struct key *key)
> > > +int tpm_tk_get_random(unsigned char *key, size_t key_len)
> > >  {
> > > -     kzfree(key->payload.data[0]);
> > > +     return tpm_get_random(chip, key, key_len);
> > >  }
> > >
> > > -struct key_type key_type_trusted = {
> > > -     .name = "trusted",
> > > -     .instantiate = trusted_instantiate,
> > > -     .update = trusted_update,
> > > -     .destroy = trusted_destroy,
> > > -     .describe = user_describe,
> > > -     .read = trusted_read,
> > > -};
> > > -
> > > -EXPORT_SYMBOL_GPL(key_type_trusted);
> > > -
> > >  static void trusted_shash_release(void)
> > >  {
> > >       if (hashalg)
> > > @@ -1182,14 +961,14 @@ static int __init trusted_shash_alloc(void)
> > >
> > >       hmacalg = crypto_alloc_shash(hmac_alg, 0, 0);
> > >       if (IS_ERR(hmacalg)) {
> > > -             pr_info("trusted_key: could not allocate crypto %s\n",
> > > +             pr_info("tpm_trusted_key: could not allocate crypto %s\n",
> > >                       hmac_alg);
> > >               return PTR_ERR(hmacalg);
> > >       }
> > >
> > >       hashalg = crypto_alloc_shash(hash_alg, 0, 0);
> > >       if (IS_ERR(hashalg)) {
> > > -             pr_info("trusted_key: could not allocate crypto %s\n",
> > > +             pr_info("tpm_trusted_key: could not allocate crypto %s\n",
> > >                       hash_alg);
> > >               ret = PTR_ERR(hashalg);
> > >               goto hashalg_fail;
> > > @@ -1217,16 +996,13 @@ static int __init init_digests(void)
> > >       return 0;
> > >  }
> > >
> > > -static int __init init_trusted(void)
> > > +static int __init init_tpm_trusted(void)
> > >  {
> > >       int ret;
> > >
> > > -     /* encrypted_keys.ko depends on successful load of this module even if
> > > -      * TPM is not used.
> > > -      */
> > >       chip = tpm_default_chip();
> > >       if (!chip)
> > > -             return 0;
> > > +             return -ENODEV;
> > >
> > >       ret = init_digests();
> > >       if (ret < 0)
> > > @@ -1247,7 +1023,7 @@ static int __init init_trusted(void)
> > >       return ret;
> > >  }
> > >
> > > -static void __exit cleanup_trusted(void)
> > > +static void __exit cleanup_tpm_trusted(void)
> > >  {
> > >       if (chip) {
> > >               put_device(&chip->dev);
> > > @@ -1257,7 +1033,12 @@ static void __exit cleanup_trusted(void)
> > >       }
> > >  }
> > >
> > > -late_initcall(init_trusted);
> > > -module_exit(cleanup_trusted);
> > > -
> > > -MODULE_LICENSE("GPL");
> > > +struct trusted_key_ops tpm_trusted_key_ops = {
> > > +     .migratable = 1, /* migratable by default */
> > > +     .init = init_tpm_trusted,
> > > +     .seal = tpm_tk_seal,
> > > +     .unseal = tpm_tk_unseal,
> > > +     .get_random = tpm_tk_get_random,
> > > +     .cleanup = cleanup_tpm_trusted,
> > > +};
> > > +EXPORT_SYMBOL_GPL(tpm_trusted_key_ops);
> >
> > Everywhere: do not use 'tk'. Use 'trusted' in those places. We do not
> > want a new acronym.
> 
> Okay.
> 
> -Sumit
> 
> >
> > /Jarkko

_______________________________________________
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] driver core: platform: expose numa_node to users in sysfs
From: Song Bao Hua (Barry Song) @ 2020-06-02  7:02 UTC (permalink / raw)
  To: Greg KH
  Cc: rafael@kernel.org, Linuxarm, linux-kernel@vger.kernel.org,
	iommu@lists.linux-foundation.org, Zengtao (B), Robin Murphy,
	linux-arm-kernel@lists.infradead.org
In-Reply-To: <20200602061112.GC2256033@kroah.com>

> >
> > On Tue, Jun 02, 2020 at 05:09:57AM +0000, Song Bao Hua (Barry Song)
> wrote:
> > > > >
> > > > > Platform devices are NUMA?  That's crazy, and feels like a total
> > > > > abuse of platform devices and drivers that really should belong
> > > > > on a
> > "real"
> > > > > bus.
> > > >
> > > > I am not sure if it is an abuse of platform device. But smmu is a
> > > > platform device, drivers/iommu/arm-smmu-v3.c is a platform driver.
> > > > In a typical ARM server, there are maybe multiple SMMU devices
> > > > which can support IO virtual address and page tables for other
> > > > devices on PCI-like busses.
> > > > Each different SMMU device might be close to different NUMA node.
> > > > There is really a hardware topology.
> > > >
> > > > If you have multiple CPU packages in a NUMA server, some platform
> > > > devices might Belong to CPU0, some other might belong to CPU1.
> > >
> > > Those devices are populated by acpi_iort for an ARM server:
> > >
> > > drivers/acpi/arm64/iort.c:
> > >
> > > static const struct iort_dev_config iort_arm_smmu_v3_cfg __initconst = {
> > >         .name = "arm-smmu-v3",
> > >         .dev_dma_configure = arm_smmu_v3_dma_configure,
> > >         .dev_count_resources = arm_smmu_v3_count_resources,
> > >         .dev_init_resources = arm_smmu_v3_init_resources,
> > >         .dev_set_proximity = arm_smmu_v3_set_proximity, };
> > >
> > > void __init acpi_iort_init(void)
> > > {
> > >         acpi_status status;
> > >
> > >         status = acpi_get_table(ACPI_SIG_IORT, 0, &iort_table);
> > >         ...
> > >         iort_check_id_count_workaround(iort_table);
> > >         iort_init_platform_devices(); }
> > >
> > > static void __init iort_init_platform_devices(void) {
> > >         ...
> > >
> > >         for (i = 0; i < iort->node_count; i++) {
> > >                 if (iort_node >= iort_end) {
> > >                         pr_err("iort node pointer overflows, bad
> > table\n");
> > >                         return;
> > >                 }
> > >
> > >                 iort_enable_acs(iort_node);
> > >
> > >                 ops = iort_get_dev_cfg(iort_node);
> > >                 if (ops) {
> > >                         fwnode = acpi_alloc_fwnode_static();
> > >                         if (!fwnode)
> > >                                 return;
> > >
> > >                         iort_set_fwnode(iort_node, fwnode);
> > >
> > >                         ret = iort_add_platform_device(iort_node,
> ops);
> > >                         if (ret) {
> > >                                 iort_delete_fwnode(iort_node);
> > >                                 acpi_free_fwnode_static(fwnode);
> > >                                 return;
> > >                         }
> > >                 }
> > >
> > >                 ...
> > >         }
> > > ...
> > > }
> > >
> > > NUMA node is got from ACPI:
> > >
> > > static int  __init arm_smmu_v3_set_proximity(struct device *dev,
> > >                                               struct
> acpi_iort_node
> > > *node) {
> > >         struct acpi_iort_smmu_v3 *smmu;
> > >
> > >         smmu = (struct acpi_iort_smmu_v3 *)node->node_data;
> > >         if (smmu->flags & ACPI_IORT_SMMU_V3_PXM_VALID) {
> > >                 int dev_node = acpi_map_pxm_to_node(smmu->pxm);
> > >
> > >                 ...
> > >
> > >                 set_dev_node(dev, dev_node);
> > >                 ...
> > >         }
> > >         return 0;
> > > }
> > >
> > > Barry
> >
> > That's fine, but those are "real" devices, not platform devices, right?
> >
> 
> Most platform devices are "real" memory-mapped hardware devices. For an
> embedded system, almost all "simple-bus"
> devices are populated from device trees as platform devices. Only a part of
> platform devices are not "real" hardware.
> 
> Smmu is a memory-mapped device. It is totally like most other platform
> devices populated in a memory space mapped in cpu's local space. It uses
> ioremap to map registers, use readl/writel to read/write its space.
> 
> > What platform device has this issue?  What one will show up this way
> > with the new patch?

Meanwhile, this patch also only shows numa_node for those platform devices with "real"
hardware and acpi support. For those platform devices which are not "real", numa_node
sys entry will not be created as dev_to_node(dev) == NUMA_NO_NODE.

For instances, 
smmu is a platform device with "real" hardware backend, it gets a numa_node like:

root@ubuntu:/sys/bus/platform/devices/arm-smmu-v3.0.auto# cat numa_node
0

root@ubuntu:/sys/bus/platform/devices/arm-smmu-v3.7.auto# cat numa_node
2

snd-soc-dummy is a platform device without "real" hardware, then it gets no numa_node:

root@ubuntu:/sys/bus/platform/devices/snd-soc-dummy# ls
driver  driver_override  modalias  power  subsystem  uevent

-barry

> 
> if platform device shouldn't be a real hardware, there is no platform device
> with a hardware topology.
> But platform devices are "real" hardware at most time. Smmu is a "real" device,
> but it is a platform device in Linux.
> 
> >
> > thanks,
> >
> > greg k-h
> 
> -barry


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [PATCH 3/3] ARM: dts: imx: Change usdhc node name on i.MX6/i.MX7 SoCs
From: Anson Huang @ 2020-06-02  6:24 UTC (permalink / raw)
  To: robh+dt, shawnguo, s.hauer, kernel, festevam, devicetree,
	linux-arm-kernel, linux-kernel
  Cc: Linux-imx
In-Reply-To: <1591079092-18625-1-git-send-email-Anson.Huang@nxp.com>

Change i.MX6/i.MX7 SoCs usdhc node name from usdhc to mmc to be
compliant with yaml schema, it requires the nodename to be "mmc".

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
 arch/arm/boot/dts/imx6qdl.dtsi | 8 ++++----
 arch/arm/boot/dts/imx6sl.dtsi  | 8 ++++----
 arch/arm/boot/dts/imx6sx.dtsi  | 8 ++++----
 arch/arm/boot/dts/imx6ul.dtsi  | 4 ++--
 arch/arm/boot/dts/imx7s.dtsi   | 6 +++---
 5 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi
index deb09df..346a52f 100644
--- a/arch/arm/boot/dts/imx6qdl.dtsi
+++ b/arch/arm/boot/dts/imx6qdl.dtsi
@@ -1056,7 +1056,7 @@
 					     <0 126 IRQ_TYPE_LEVEL_HIGH>;
 			};
 
-			usdhc1: usdhc@2190000 {
+			usdhc1: mmc@2190000 {
 				compatible = "fsl,imx6q-usdhc";
 				reg = <0x02190000 0x4000>;
 				interrupts = <0 22 IRQ_TYPE_LEVEL_HIGH>;
@@ -1068,7 +1068,7 @@
 				status = "disabled";
 			};
 
-			usdhc2: usdhc@2194000 {
+			usdhc2: mmc@2194000 {
 				compatible = "fsl,imx6q-usdhc";
 				reg = <0x02194000 0x4000>;
 				interrupts = <0 23 IRQ_TYPE_LEVEL_HIGH>;
@@ -1080,7 +1080,7 @@
 				status = "disabled";
 			};
 
-			usdhc3: usdhc@2198000 {
+			usdhc3: mmc@2198000 {
 				compatible = "fsl,imx6q-usdhc";
 				reg = <0x02198000 0x4000>;
 				interrupts = <0 24 IRQ_TYPE_LEVEL_HIGH>;
@@ -1092,7 +1092,7 @@
 				status = "disabled";
 			};
 
-			usdhc4: usdhc@219c000 {
+			usdhc4: mmc@219c000 {
 				compatible = "fsl,imx6q-usdhc";
 				reg = <0x0219c000 0x4000>;
 				interrupts = <0 25 IRQ_TYPE_LEVEL_HIGH>;
diff --git a/arch/arm/boot/dts/imx6sl.dtsi b/arch/arm/boot/dts/imx6sl.dtsi
index 11e7bf3..e2d2532 100644
--- a/arch/arm/boot/dts/imx6sl.dtsi
+++ b/arch/arm/boot/dts/imx6sl.dtsi
@@ -854,7 +854,7 @@
 				status = "disabled";
 			};
 
-			usdhc1: usdhc@2190000 {
+			usdhc1: mmc@2190000 {
 				compatible = "fsl,imx6sl-usdhc", "fsl,imx6q-usdhc";
 				reg = <0x02190000 0x4000>;
 				interrupts = <0 22 IRQ_TYPE_LEVEL_HIGH>;
@@ -866,7 +866,7 @@
 				status = "disabled";
 			};
 
-			usdhc2: usdhc@2194000 {
+			usdhc2: mmc@2194000 {
 				compatible = "fsl,imx6sl-usdhc", "fsl,imx6q-usdhc";
 				reg = <0x02194000 0x4000>;
 				interrupts = <0 23 IRQ_TYPE_LEVEL_HIGH>;
@@ -878,7 +878,7 @@
 				status = "disabled";
 			};
 
-			usdhc3: usdhc@2198000 {
+			usdhc3: mmc@2198000 {
 				compatible = "fsl,imx6sl-usdhc", "fsl,imx6q-usdhc";
 				reg = <0x02198000 0x4000>;
 				interrupts = <0 24 IRQ_TYPE_LEVEL_HIGH>;
@@ -890,7 +890,7 @@
 				status = "disabled";
 			};
 
-			usdhc4: usdhc@219c000 {
+			usdhc4: mmc@219c000 {
 				compatible = "fsl,imx6sl-usdhc", "fsl,imx6q-usdhc";
 				reg = <0x0219c000 0x4000>;
 				interrupts = <0 25 IRQ_TYPE_LEVEL_HIGH>;
diff --git a/arch/arm/boot/dts/imx6sx.dtsi b/arch/arm/boot/dts/imx6sx.dtsi
index 995e1b1..430c21a 100644
--- a/arch/arm/boot/dts/imx6sx.dtsi
+++ b/arch/arm/boot/dts/imx6sx.dtsi
@@ -943,7 +943,7 @@
 				status = "disabled";
 			};
 
-			usdhc1: usdhc@2190000 {
+			usdhc1: mmc@2190000 {
 				compatible = "fsl,imx6sx-usdhc", "fsl,imx6sl-usdhc";
 				reg = <0x02190000 0x4000>;
 				interrupts = <GIC_SPI 22 IRQ_TYPE_LEVEL_HIGH>;
@@ -955,7 +955,7 @@
 				status = "disabled";
 			};
 
-			usdhc2: usdhc@2194000 {
+			usdhc2: mmc@2194000 {
 				compatible = "fsl,imx6sx-usdhc", "fsl,imx6sl-usdhc";
 				reg = <0x02194000 0x4000>;
 				interrupts = <GIC_SPI 23 IRQ_TYPE_LEVEL_HIGH>;
@@ -967,7 +967,7 @@
 				status = "disabled";
 			};
 
-			usdhc3: usdhc@2198000 {
+			usdhc3: mmc@2198000 {
 				compatible = "fsl,imx6sx-usdhc", "fsl,imx6sl-usdhc";
 				reg = <0x02198000 0x4000>;
 				interrupts = <GIC_SPI 24 IRQ_TYPE_LEVEL_HIGH>;
@@ -979,7 +979,7 @@
 				status = "disabled";
 			};
 
-			usdhc4: usdhc@219c000 {
+			usdhc4: mmc@219c000 {
 				compatible = "fsl,imx6sx-usdhc", "fsl,imx6sl-usdhc";
 				reg = <0x0219c000 0x4000>;
 				interrupts = <GIC_SPI 25 IRQ_TYPE_LEVEL_HIGH>;
diff --git a/arch/arm/boot/dts/imx6ul.dtsi b/arch/arm/boot/dts/imx6ul.dtsi
index 2d64802..e5aab37 100644
--- a/arch/arm/boot/dts/imx6ul.dtsi
+++ b/arch/arm/boot/dts/imx6ul.dtsi
@@ -861,7 +861,7 @@
 				status = "disabled";
 			};
 
-			usdhc1: usdhc@2190000 {
+			usdhc1: mmc@2190000 {
 				compatible = "fsl,imx6ul-usdhc", "fsl,imx6sx-usdhc";
 				reg = <0x02190000 0x4000>;
 				interrupts = <GIC_SPI 22 IRQ_TYPE_LEVEL_HIGH>;
@@ -875,7 +875,7 @@
 				status = "disabled";
 			};
 
-			usdhc2: usdhc@2194000 {
+			usdhc2: mmc@2194000 {
 				compatible = "fsl,imx6ul-usdhc", "fsl,imx6sx-usdhc";
 				reg = <0x02194000 0x4000>;
 				interrupts = <GIC_SPI 23 IRQ_TYPE_LEVEL_HIGH>;
diff --git a/arch/arm/boot/dts/imx7s.dtsi b/arch/arm/boot/dts/imx7s.dtsi
index 3cba731..1cfaf41 100644
--- a/arch/arm/boot/dts/imx7s.dtsi
+++ b/arch/arm/boot/dts/imx7s.dtsi
@@ -1126,7 +1126,7 @@
 				reg = <0x30b30200 0x200>;
 			};
 
-			usdhc1: usdhc@30b40000 {
+			usdhc1: mmc@30b40000 {
 				compatible = "fsl,imx7d-usdhc", "fsl,imx6sl-usdhc";
 				reg = <0x30b40000 0x10000>;
 				interrupts = <GIC_SPI 22 IRQ_TYPE_LEVEL_HIGH>;
@@ -1138,7 +1138,7 @@
 				status = "disabled";
 			};
 
-			usdhc2: usdhc@30b50000 {
+			usdhc2: mmc@30b50000 {
 				compatible = "fsl,imx7d-usdhc", "fsl,imx6sl-usdhc";
 				reg = <0x30b50000 0x10000>;
 				interrupts = <GIC_SPI 23 IRQ_TYPE_LEVEL_HIGH>;
@@ -1150,7 +1150,7 @@
 				status = "disabled";
 			};
 
-			usdhc3: usdhc@30b60000 {
+			usdhc3: mmc@30b60000 {
 				compatible = "fsl,imx7d-usdhc", "fsl,imx6sl-usdhc";
 				reg = <0x30b60000 0x10000>;
 				interrupts = <GIC_SPI 24 IRQ_TYPE_LEVEL_HIGH>;
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [PATCH 2/3] ARM: dts: imx: Change esdhc node name on i.MX2/i.MX3/i.MX5 SoCs
From: Anson Huang @ 2020-06-02  6:24 UTC (permalink / raw)
  To: robh+dt, shawnguo, s.hauer, kernel, festevam, devicetree,
	linux-arm-kernel, linux-kernel
  Cc: Linux-imx
In-Reply-To: <1591079092-18625-1-git-send-email-Anson.Huang@nxp.com>

Change i.MX2/i.MX3/i.MX5 SoCs esdhc node name from esdhc to mmc to
be compliant with yaml schema, it requires the nodename to be "mmc".

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
 arch/arm/boot/dts/imx25.dtsi | 4 ++--
 arch/arm/boot/dts/imx35.dtsi | 6 +++---
 arch/arm/boot/dts/imx50.dtsi | 8 ++++----
 arch/arm/boot/dts/imx51.dtsi | 8 ++++----
 arch/arm/boot/dts/imx53.dtsi | 8 ++++----
 5 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/arch/arm/boot/dts/imx25.dtsi b/arch/arm/boot/dts/imx25.dtsi
index 4eaf4eb..b045c6d 100644
--- a/arch/arm/boot/dts/imx25.dtsi
+++ b/arch/arm/boot/dts/imx25.dtsi
@@ -453,7 +453,7 @@
 				interrupts = <22>;
 			};
 
-			esdhc1: esdhc@53fb4000 {
+			esdhc1: mmc@53fb4000 {
 				compatible = "fsl,imx25-esdhc";
 				reg = <0x53fb4000 0x4000>;
 				interrupts = <9>;
@@ -462,7 +462,7 @@
 				status = "disabled";
 			};
 
-			esdhc2: esdhc@53fb8000 {
+			esdhc2: mmc@53fb8000 {
 				compatible = "fsl,imx25-esdhc";
 				reg = <0x53fb8000 0x4000>;
 				interrupts = <8>;
diff --git a/arch/arm/boot/dts/imx35.dtsi b/arch/arm/boot/dts/imx35.dtsi
index 502112b..e154087 100644
--- a/arch/arm/boot/dts/imx35.dtsi
+++ b/arch/arm/boot/dts/imx35.dtsi
@@ -231,7 +231,7 @@
 				#interrupt-cells = <2>;
 			};
 
-			esdhc1: esdhc@53fb4000 {
+			esdhc1: mmc@53fb4000 {
 				compatible = "fsl,imx35-esdhc";
 				reg = <0x53fb4000 0x4000>;
 				interrupts = <7>;
@@ -240,7 +240,7 @@
 				status = "disabled";
 			};
 
-			esdhc2: esdhc@53fb8000 {
+			esdhc2: mmc@53fb8000 {
 				compatible = "fsl,imx35-esdhc";
 				reg = <0x53fb8000 0x4000>;
 				interrupts = <8>;
@@ -249,7 +249,7 @@
 				status = "disabled";
 			};
 
-			esdhc3: esdhc@53fbc000 {
+			esdhc3: mmc@53fbc000 {
 				compatible = "fsl,imx35-esdhc";
 				reg = <0x53fbc000 0x4000>;
 				interrupts = <9>;
diff --git a/arch/arm/boot/dts/imx50.dtsi b/arch/arm/boot/dts/imx50.dtsi
index 1f4ecbc..377951a 100644
--- a/arch/arm/boot/dts/imx50.dtsi
+++ b/arch/arm/boot/dts/imx50.dtsi
@@ -115,7 +115,7 @@
 				reg = <0x50000000 0x40000>;
 				ranges;
 
-				esdhc1: esdhc@50004000 {
+				esdhc1: mmc@50004000 {
 					compatible = "fsl,imx50-esdhc", "fsl,imx53-esdhc";
 					reg = <0x50004000 0x4000>;
 					interrupts = <1>;
@@ -127,7 +127,7 @@
 					status = "disabled";
 				};
 
-				esdhc2: esdhc@50008000 {
+				esdhc2: mmc@50008000 {
 					compatible = "fsl,imx50-esdhc", "fsl,imx53-esdhc";
 					reg = <0x50008000 0x4000>;
 					interrupts = <2>;
@@ -176,7 +176,7 @@
 					status = "disabled";
 				};
 
-				esdhc3: esdhc@50020000 {
+				esdhc3: mmc@50020000 {
 					compatible = "fsl,imx50-esdhc", "fsl,imx53-esdhc";
 					reg = <0x50020000 0x4000>;
 					interrupts = <3>;
@@ -188,7 +188,7 @@
 					status = "disabled";
 				};
 
-				esdhc4: esdhc@50024000 {
+				esdhc4: mmc@50024000 {
 					compatible = "fsl,imx50-esdhc", "fsl,imx53-esdhc";
 					reg = <0x50024000 0x4000>;
 					interrupts = <4>;
diff --git a/arch/arm/boot/dts/imx51.dtsi b/arch/arm/boot/dts/imx51.dtsi
index c83bc77..db5827d 100644
--- a/arch/arm/boot/dts/imx51.dtsi
+++ b/arch/arm/boot/dts/imx51.dtsi
@@ -185,7 +185,7 @@
 				reg = <0x70000000 0x40000>;
 				ranges;
 
-				esdhc1: esdhc@70004000 {
+				esdhc1: mmc@70004000 {
 					compatible = "fsl,imx51-esdhc";
 					reg = <0x70004000 0x4000>;
 					interrupts = <1>;
@@ -196,7 +196,7 @@
 					status = "disabled";
 				};
 
-				esdhc2: esdhc@70008000 {
+				esdhc2: mmc@70008000 {
 					compatible = "fsl,imx51-esdhc";
 					reg = <0x70008000 0x4000>;
 					interrupts = <2>;
@@ -245,7 +245,7 @@
 					status = "disabled";
 				};
 
-				esdhc3: esdhc@70020000 {
+				esdhc3: mmc@70020000 {
 					compatible = "fsl,imx51-esdhc";
 					reg = <0x70020000 0x4000>;
 					interrupts = <3>;
@@ -257,7 +257,7 @@
 					status = "disabled";
 				};
 
-				esdhc4: esdhc@70024000 {
+				esdhc4: mmc@70024000 {
 					compatible = "fsl,imx51-esdhc";
 					reg = <0x70024000 0x4000>;
 					interrupts = <4>;
diff --git a/arch/arm/boot/dts/imx53.dtsi b/arch/arm/boot/dts/imx53.dtsi
index ed6b0c8..9a4fc99 100644
--- a/arch/arm/boot/dts/imx53.dtsi
+++ b/arch/arm/boot/dts/imx53.dtsi
@@ -236,7 +236,7 @@
 				reg = <0x50000000 0x40000>;
 				ranges;
 
-				esdhc1: esdhc@50004000 {
+				esdhc1: mmc@50004000 {
 					compatible = "fsl,imx53-esdhc";
 					reg = <0x50004000 0x4000>;
 					interrupts = <1>;
@@ -248,7 +248,7 @@
 					status = "disabled";
 				};
 
-				esdhc2: esdhc@50008000 {
+				esdhc2: mmc@50008000 {
 					compatible = "fsl,imx53-esdhc";
 					reg = <0x50008000 0x4000>;
 					interrupts = <2>;
@@ -301,7 +301,7 @@
 					status = "disabled";
 				};
 
-				esdhc3: esdhc@50020000 {
+				esdhc3: mmc@50020000 {
 					compatible = "fsl,imx53-esdhc";
 					reg = <0x50020000 0x4000>;
 					interrupts = <3>;
@@ -313,7 +313,7 @@
 					status = "disabled";
 				};
 
-				esdhc4: esdhc@50024000 {
+				esdhc4: mmc@50024000 {
 					compatible = "fsl,imx53-esdhc";
 					reg = <0x50024000 0x4000>;
 					interrupts = <4>;
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [PATCH 1/3] ARM: dts: imx: Change sdhci node name on i.MX27/i.MX31 SoCs
From: Anson Huang @ 2020-06-02  6:24 UTC (permalink / raw)
  To: robh+dt, shawnguo, s.hauer, kernel, festevam, devicetree,
	linux-arm-kernel, linux-kernel
  Cc: Linux-imx

Change i.MX27/i.MX31 node name from sdhci to mmc to be compliant
with yaml schema, it requires the nodename to be "mmc".

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
 arch/arm/boot/dts/imx27.dtsi | 6 +++---
 arch/arm/boot/dts/imx31.dtsi | 4 ++--
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/arm/boot/dts/imx27.dtsi b/arch/arm/boot/dts/imx27.dtsi
index ee04771..47de96b 100644
--- a/arch/arm/boot/dts/imx27.dtsi
+++ b/arch/arm/boot/dts/imx27.dtsi
@@ -265,7 +265,7 @@
 				status = "disabled";
 			};
 
-			sdhci1: sdhci@10013000 {
+			sdhci1: mmc@10013000 {
 				compatible = "fsl,imx27-mmc", "fsl,imx21-mmc";
 				reg = <0x10013000 0x1000>;
 				interrupts = <11>;
@@ -277,7 +277,7 @@
 				status = "disabled";
 			};
 
-			sdhci2: sdhci@10014000 {
+			sdhci2: mmc@10014000 {
 				compatible = "fsl,imx27-mmc", "fsl,imx21-mmc";
 				reg = <0x10014000 0x1000>;
 				interrupts = <10>;
@@ -431,7 +431,7 @@
 				status = "disabled";
 			};
 
-			sdhci3: sdhci@1001e000 {
+			sdhci3: mmc@1001e000 {
 				compatible = "fsl,imx27-mmc", "fsl,imx21-mmc";
 				reg = <0x1001e000 0x1000>;
 				interrupts = <9>;
diff --git a/arch/arm/boot/dts/imx31.dtsi b/arch/arm/boot/dts/imx31.dtsi
index 4f3d7ab..eedf2d7 100644
--- a/arch/arm/boot/dts/imx31.dtsi
+++ b/arch/arm/boot/dts/imx31.dtsi
@@ -173,7 +173,7 @@
 			reg = <0x50000000 0x100000>;
 			ranges;
 
-			sdhci1: sdhci@50004000 {
+			sdhci1: mmc@50004000 {
 				compatible = "fsl,imx31-mmc";
 				reg = <0x50004000 0x4000>;
 				interrupts = <9>;
@@ -184,7 +184,7 @@
 				status = "disabled";
 			};
 
-			sdhci2: sdhci@50008000 {
+			sdhci2: mmc@50008000 {
 				compatible = "fsl,imx31-mmc";
 				reg = <0x50008000 0x4000>;
 				interrupts = <8>;
-- 
2.7.4


_______________________________________________
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 0/2] iommu: arm-smmu: Add support for early direct mappings
From: Bjorn Andersson @ 2020-06-02  6:32 UTC (permalink / raw)
  To: Will Deacon
  Cc: linux-arm-msm, Joerg Roedel, iommu, Thierry Reding, John Stultz,
	linux-tegra, Robin Murphy, linux-arm-kernel
In-Reply-To: <20200527110343.GD11111@willie-the-truck>

On Wed 27 May 04:03 PDT 2020, Will Deacon wrote:

> Hi John, Bjorn,
> 
> On Tue, May 26, 2020 at 01:34:45PM -0700, John Stultz wrote:
> > On Thu, May 14, 2020 at 12:34 PM <bjorn.andersson@linaro.org> wrote:
> > >
> > > On Thu 27 Feb 18:57 PST 2020, Bjorn Andersson wrote:
> > >
> > > Rob, Will, we're reaching the point where upstream has enough
> > > functionality that this is becoming a critical issue for us.
> > >
> > > E.g. Lenovo Yoga C630 is lacking this and a single dts patch to boot
> > > mainline with display, GPU, WiFi and audio working and the story is
> > > similar on several devboards.
> > >
> > > As previously described, the only thing I want is the stream mapping
> > > related to the display controller in place, either with the CB with
> > > translation disabled or possibly with a way to specify the framebuffer
> > > region (although this turns out to mess things up in the display
> > > driver...)
> > >
> > > I did pick this up again recently and concluded that by omitting the
> > > streams for the USB controllers causes an instability issue seen on one
> > > of the controller to disappear. So I would prefer if we somehow could
> > > have a mechanism to only pick the display streams and the context
> > > allocation for this.
> > >
> > >
> > > Can you please share some pointers/insights/wishes for how we can
> > > conclude on this subject?
> > 
> > Ping? I just wanted to follow up on this discussion as this small
> > series is crucial for booting mainline on the Dragonboard 845c
> > devboard. It would be really valuable to be able to get some solution
> > upstream so we can test mainline w/o adding additional patches.
> 
> Sorry, it's been insanely busy recently and I haven't had a chance to think
> about this on top of everything else. We're also carrying a hack in Android
> for you :)
> 

Thanks for taking the time to get back to us on this!

> > The rest of the db845c series has been moving forward smoothly, but
> > this set seems to be very stuck with no visible progress since Dec.
> > 
> > Are there any pointers for what folks would prefer to see?
> 
> I've had a chat with Robin about this. Originally, I was hoping that
> people would all work together towards an idyllic future where firmware
> would be able to describe arbitrary pre-existing mappings for devices,
> irrespective of the IOMMU through which they master and Linux could
> inherit this configuration. However, that hasn't materialised (there was
> supposed to be an IORT update, but I don't know what happened to that)
> and, in actual fact, the problem that you have on db845 is /far/ more
> restricted than the general problem.
> 
> Could you please try hacking something along the following lines and see
> how you get on? You may need my for-joerg/arm-smmu/updates branch for
> all the pieces:
> 
>   1. Use the ->cfg_probe() callback to reserve the SMR/S2CRs you need
>      "pinning" and configure for bypass.
> 
>   2. Use the ->def_domain_type() callback to return IOMMU_DOMAIN_IDENTITY
>      for the display controller
> 
> I /think/ that's sufficient, but note that it differs from the current
> approach because we don't end up reserving a CB -- bypass is configured
> in the S2CR instead. Some invalidation might therefore be needed in
> ->cfg_probe() after unhooking the CB.
> 
> Thanks, and please yell if you run into problems with this approach.
> 

This sounded straight forward and cleaner, so I implemented it...

Unfortunately the hypervisor is playing tricks on me when writing to
S2CR registers:
- TRANS writes lands as requested
- BYPASS writes ends up in the register as requested, with type FAULT
- FAULT writes are ignored

In other words, the Qualcomm firmware prevents us from relying on
marking the relevant streams as BYPASS type.


Instead Qualcomm seems to implement "bypass" by setting up stream
mapping, of TRANS type, pointing to a context bank without
ARM_SMMU_SCTLR_M set.

Regards,
Bjorn

_______________________________________________
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] driver core: platform: expose numa_node to users in sysfs
From: Song Bao Hua (Barry Song) @ 2020-06-02  6:26 UTC (permalink / raw)
  To: Greg KH
  Cc: rafael@kernel.org, Linuxarm, linux-kernel@vger.kernel.org,
	iommu@lists.linux-foundation.org, Zengtao (B), Robin Murphy,
	linux-arm-kernel@lists.infradead.org
In-Reply-To: <20200602061112.GC2256033@kroah.com>



> -----Original Message-----
> From: Greg KH [mailto:gregkh@linuxfoundation.org]
> Sent: Tuesday, June 2, 2020 6:11 PM
> To: Song Bao Hua (Barry Song) <song.bao.hua@hisilicon.com>
> Cc: rafael@kernel.org; iommu@lists.linux-foundation.org;
> linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org; Linuxarm
> <linuxarm@huawei.com>; Zengtao (B) <prime.zeng@hisilicon.com>; Robin
> Murphy <robin.murphy@arm.com>
> Subject: Re: [PATCH] driver core: platform: expose numa_node to users in sysfs
> 
> On Tue, Jun 02, 2020 at 05:09:57AM +0000, Song Bao Hua (Barry Song) wrote:
> > > >
> > > > Platform devices are NUMA?  That's crazy, and feels like a total
> > > > abuse of platform devices and drivers that really should belong on a
> "real"
> > > > bus.
> > >
> > > I am not sure if it is an abuse of platform device. But smmu is a
> > > platform device, drivers/iommu/arm-smmu-v3.c is a platform driver.
> > > In a typical ARM server, there are maybe multiple SMMU devices which
> > > can support IO virtual address and page tables for other devices on
> > > PCI-like busses.
> > > Each different SMMU device might be close to different NUMA node.
> > > There is really a hardware topology.
> > >
> > > If you have multiple CPU packages in a NUMA server, some platform
> > > devices might Belong to CPU0, some other might belong to CPU1.
> >
> > Those devices are populated by acpi_iort for an ARM server:
> >
> > drivers/acpi/arm64/iort.c:
> >
> > static const struct iort_dev_config iort_arm_smmu_v3_cfg __initconst = {
> >         .name = "arm-smmu-v3",
> >         .dev_dma_configure = arm_smmu_v3_dma_configure,
> >         .dev_count_resources = arm_smmu_v3_count_resources,
> >         .dev_init_resources = arm_smmu_v3_init_resources,
> >         .dev_set_proximity = arm_smmu_v3_set_proximity, };
> >
> > void __init acpi_iort_init(void)
> > {
> >         acpi_status status;
> >
> >         status = acpi_get_table(ACPI_SIG_IORT, 0, &iort_table);
> >         ...
> >         iort_check_id_count_workaround(iort_table);
> >         iort_init_platform_devices();
> > }
> >
> > static void __init iort_init_platform_devices(void) {
> >         ...
> >
> >         for (i = 0; i < iort->node_count; i++) {
> >                 if (iort_node >= iort_end) {
> >                         pr_err("iort node pointer overflows, bad
> table\n");
> >                         return;
> >                 }
> >
> >                 iort_enable_acs(iort_node);
> >
> >                 ops = iort_get_dev_cfg(iort_node);
> >                 if (ops) {
> >                         fwnode = acpi_alloc_fwnode_static();
> >                         if (!fwnode)
> >                                 return;
> >
> >                         iort_set_fwnode(iort_node, fwnode);
> >
> >                         ret = iort_add_platform_device(iort_node, ops);
> >                         if (ret) {
> >                                 iort_delete_fwnode(iort_node);
> >                                 acpi_free_fwnode_static(fwnode);
> >                                 return;
> >                         }
> >                 }
> >
> >                 ...
> >         }
> > ...
> > }
> >
> > NUMA node is got from ACPI:
> >
> > static int  __init arm_smmu_v3_set_proximity(struct device *dev,
> >                                               struct acpi_iort_node
> > *node) {
> >         struct acpi_iort_smmu_v3 *smmu;
> >
> >         smmu = (struct acpi_iort_smmu_v3 *)node->node_data;
> >         if (smmu->flags & ACPI_IORT_SMMU_V3_PXM_VALID) {
> >                 int dev_node = acpi_map_pxm_to_node(smmu->pxm);
> >
> >                 ...
> >
> >                 set_dev_node(dev, dev_node);
> >                 ...
> >         }
> >         return 0;
> > }
> >
> > Barry
> 
> That's fine, but those are "real" devices, not platform devices, right?
> 

Most platform devices are "real" memory-mapped hardware devices. For an embedded system, almost all "simple-bus"
devices are populated from device trees as platform devices. Only a part of platform devices are not "real" hardware.

Smmu is a memory-mapped device. It is totally like most other platform devices populated in a 
memory space mapped in cpu's local space. It uses ioremap to map registers, use readl/writel to read/write its
space.

> What platform device has this issue?  What one will show up this way with
> the new patch?

if platform device shouldn't be a real hardware, there is no platform device with a hardware topology.
But platform devices are "real" hardware at most time. Smmu is a "real" device, but it is a platform device in Linux.

> 
> thanks,
> 
> greg k-h

-barry


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [V9, 1/2] media: dt-bindings: media: i2c: Document OV02A10 bindings
From: Dongchun Zhu @ 2020-06-02  6:15 UTC (permalink / raw)
  To: Tomasz Figa
  Cc: Mark Rutland, Rob Herring, Andy Shevchenko, srv_heupstream,
	linux-devicetree, Linus Walleij,
	Shengnan Wang (王圣男), Louis Kuo,
	Bartosz Golaszewski, Sj Huang, Nicolas Boichat,
	moderated list:ARM/Mediatek SoC support, Sakari Ailus,
	Matthias Brugger, Cao Bing Bu, Mauro Carvalho Chehab,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	Linux Media Mailing List
In-Reply-To: <CAAFQd5AY9gejoiwxojvvG0FaVfEAf8gCqOddvo-dxemQWFksVw@mail.gmail.com>

Hi Tomasz, Sakari,

On Mon, 2020-06-01 at 20:18 +0200, Tomasz Figa wrote:
> On Mon, Jun 1, 2020 at 4:35 AM Dongchun Zhu <dongchun.zhu@mediatek.com> wrote:
> >
> > Hi Tomasz,
> >
> > On Fri, 2020-05-29 at 15:43 +0200, Tomasz Figa wrote:
> > > On Thu, May 28, 2020 at 10:06 AM Dongchun Zhu <dongchun.zhu@mediatek.com> wrote:
> > > >
> > > > Hi Sakari,
> > > >
> > > > On Thu, 2020-05-28 at 10:23 +0300, Sakari Ailus wrote:
> > > > > Hi Dongchun,
> > > > >
> > > > > On Thu, May 28, 2020 at 11:34:42AM +0800, Dongchun Zhu wrote:
> > > > > > Hi Sakari, Rob,
> > > > > >
> > > > > > On Thu, 2020-05-28 at 00:16 +0300, Sakari Ailus wrote:
> > > > > > > Hi Rob, Dongchun,
> > > > > > >
> > > > > > > On Wed, May 27, 2020 at 09:27:22AM -0600, Rob Herring wrote:
> > > > > > > > > > > +    properties:
> > > > > > > > > > > +      endpoint:
> > > > > > > > > > > +        type: object
> > > > > > > > > > > +        additionalProperties: false
> > > > > > > > > > > +
> > > > > > > > > > > +        properties:
> > > > > > > > >
> > > > > > > > > Actually I wonder whether we need to declare 'clock-lanes' here?
> > > > > > > >
> > > > > > > > Yes, if you are using it.
> > > > > > >
> > > > > > > Dongchun, can you confirm the chip has a single data and a single clock
> > > > > > > lane and that it does not support lane reordering?
> > > > > > >
> > > > > >
> > > > > > From the datasheet, 'MIPI inside the OV02A10 provides one single
> > > > > > uni-directional clock lane and one bi-directional data lane solution for
> > > > > > communication links between components inside a mobile device.
> > > > > > The data lane has full support for HS(uni-directional) and
> > > > > > LP(bi-directional) data transfer mode.'
> > > > > >
> > > > > > The sensor doesn't support lane reordering, so 'clock-lanes' property
> > > > > > would not be added in next release.
> > > > > >
> > > > > > > So if there's nothing to convey to the driver, also the data-lanes should
> > > > > > > be removed IMO.
> > > > > > >
> > > > > >
> > > > > > However, 'data-lanes' property may still be required.
> > > > > > It is known that either data-lanes or clock-lanes is an array of
> > > > > > physical data lane indexes. Position of an entry determines the logical
> > > > > > lane number, while the value of an entry indicates physical lane, e.g.,
> > > > > > for 1-lane MIPI CSI-2 bus we could have "data-lanes = <1>;", assuming
> > > > > > the clock lane is on hardware lane 0.
> > > > > >
> > > > > > As mentioned earlier, the OV02A10 sensor supports only 1C1D and does not
> > > > > > support lane reordering, so here we shall use 'data-lanes = <1>' as
> > > > > > there is only a clock lane for OV02A10.
> > > > > >
> > > > > > Reminder:
> > > > > > If 'data-lanes' property is not present, the driver would assume
> > > > > > four-lane operation. This means for one-lane or two-lane operation, this
> > > > > > property must be present and set to the right physical lane indexes.
> > > > > > If the hardware does not support lane reordering, monotonically
> > > > > > incremented values shall be used from 0 or 1 onwards, depending on
> > > > > > whether or not there is also a clock lane.
> > > > >
> > > > > How can the driver use four lanes, considering the device only supports a
> > > > > single lane??
> > > > >
> > > >
> > > > I understood your meaning.
> > > > If we omit the property 'data-lanes', the sensor should work still.
> > > > But then what's the meaning of the existence of 'data-lanes'?
> > > > If this property 'data-lanes' is always optional, then why dt-bindings
> > > > provide the interface?
> > > >
> > > > In the meantime, if omitting 'data-lanes' for one sensor(transmitter)
> > > > that has only one physical data lane, MIPI receiver(e.g., MIPI CSI-2)
> > > > shall enable four-lane configuration, which may increase consumption of
> > > > both power and resource in the process of IIC communication.
> > >
> > > Wouldn't the receiver still have the data-lanes property under its
> > > endpoint node, telling it how many lanes and in which order should be
> > > used?
> > >
> >
> > The MIPI receiver(RX) shall use
> > v4l2_async_notifier_add_fwnode_remote_subdev() API to parse the property
> > "data-lanes" under sensor output port.
> 
> That's not true. The MIPI receiver driver parses its own port node
> corresponding to the sensor. Also quoting the documentation [1]:
> 
> "An endpoint subnode of a device contains all properties needed for
> _configuration of this device_ for data exchange with other device. In most
> cases properties at the peer 'endpoint' nodes will be identical, however they
> might need to be different when there is any signal modifications on the bus
> between two devices, e.g. there are logic signal inverters on the lines."
> 
> In this case, there is such a signal modification if the sensor has a
> 1-lane bus and the receiver more lines, so the data-lanes properties
> would be different on both sides.
> 
> [1] https://elixir.bootlin.com/linux/v5.7/source/Documentation/devicetree/bindings/media/video-interfaces.txt
> 

Sorry for the misunderstanding.
After doing some experiments about the data-lanes property under sensor
i2c node, we found the API
v4l2_async_notifier_add_fwnode_remote_subdev() that MIPI receiver driver
used indeed parses the data-lanes under its own port node.

Sorry make a mistake for the use case of sensor data-lanes previously.
Now We may encounter one new question for this patch.
In practice we haven't used the data-lanes under sensor i2c node
anywhere, if sensor driver itself doesn't parse that.

But there is still one reason to keep the exactly right data-lanes in
DT. That is, the data-lanes under sensor i2c node could be used as a
reference for MIPI receiver driver.
Just as Tomasz said, 'The MIPI receiver driver parses its own port node
corresponding to the sensor'.

Sakari, Tomasz, what's your opinions about the present of data-lanes
under sensor node or not?

> Best regards,
> Tomasz

_______________________________________________
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] driver core: platform: expose numa_node to users in sysfs
From: Greg KH @ 2020-06-02  6:11 UTC (permalink / raw)
  To: Song Bao Hua (Barry Song)
  Cc: rafael@kernel.org, Linuxarm, linux-kernel@vger.kernel.org,
	iommu@lists.linux-foundation.org, Zengtao (B), Robin Murphy,
	linux-arm-kernel@lists.infradead.org
In-Reply-To: <B926444035E5E2439431908E3842AFD24D8F9E@DGGEMI525-MBS.china.huawei.com>

On Tue, Jun 02, 2020 at 05:09:57AM +0000, Song Bao Hua (Barry Song) wrote:
> > >
> > > Platform devices are NUMA?  That's crazy, and feels like a total abuse
> > > of platform devices and drivers that really should belong on a "real"
> > > bus.
> > 
> > I am not sure if it is an abuse of platform device. But smmu is a platform
> > device,
> > drivers/iommu/arm-smmu-v3.c is a platform driver.
> > In a typical ARM server, there are maybe multiple SMMU devices which can
> > support
> > IO virtual address and page tables for other devices on PCI-like busses.
> > Each different SMMU device might be close to different NUMA node. There is
> > really a hardware topology.
> > 
> > If you have multiple CPU packages in a NUMA server, some platform devices
> > might
> > Belong to CPU0, some other might belong to CPU1.
> 
> Those devices are populated by acpi_iort for an ARM server:
> 
> drivers/acpi/arm64/iort.c:
> 
> static const struct iort_dev_config iort_arm_smmu_v3_cfg __initconst = {
>         .name = "arm-smmu-v3",
>         .dev_dma_configure = arm_smmu_v3_dma_configure,
>         .dev_count_resources = arm_smmu_v3_count_resources,
>         .dev_init_resources = arm_smmu_v3_init_resources,
>         .dev_set_proximity = arm_smmu_v3_set_proximity,
> };
> 
> void __init acpi_iort_init(void)
> {
>         acpi_status status;
> 
>         status = acpi_get_table(ACPI_SIG_IORT, 0, &iort_table);
>         ...
>         iort_check_id_count_workaround(iort_table);
>         iort_init_platform_devices();
> }
> 
> static void __init iort_init_platform_devices(void)
> {
>         ...
> 
>         for (i = 0; i < iort->node_count; i++) {
>                 if (iort_node >= iort_end) {
>                         pr_err("iort node pointer overflows, bad table\n");
>                         return;
>                 }
> 
>                 iort_enable_acs(iort_node);
> 
>                 ops = iort_get_dev_cfg(iort_node);
>                 if (ops) {
>                         fwnode = acpi_alloc_fwnode_static();
>                         if (!fwnode)
>                                 return;
> 
>                         iort_set_fwnode(iort_node, fwnode);
> 
>                         ret = iort_add_platform_device(iort_node, ops);
>                         if (ret) {
>                                 iort_delete_fwnode(iort_node);
>                                 acpi_free_fwnode_static(fwnode);
>                                 return;
>                         }
>                 }
> 
>                 ...
>         }
> ...
> }
> 
> NUMA node is got from ACPI:
> 
> static int  __init arm_smmu_v3_set_proximity(struct device *dev,
>                                               struct acpi_iort_node *node)
> {
>         struct acpi_iort_smmu_v3 *smmu;
> 
>         smmu = (struct acpi_iort_smmu_v3 *)node->node_data;
>         if (smmu->flags & ACPI_IORT_SMMU_V3_PXM_VALID) {
>                 int dev_node = acpi_map_pxm_to_node(smmu->pxm);
> 
>                 ...
> 
>                 set_dev_node(dev, dev_node);
>                 ...
>         }
>         return 0;
> }
> 
> Barry

That's fine, but those are "real" devices, not platform devices, right?

What platform device has this issue?  What one will show up this way
with the new patch?

thanks,

greg k-h

_______________________________________________
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] iio: Kconfig: at91_adc: add COMPILE_TEST dependency to driver
From: Ludovic Desroches @ 2020-06-02  6:06 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: alexandre.belloni, linux-iio, linux-kernel, Alexandru Ardelean,
	linux-arm-kernel
In-Reply-To: <20200531154017.04fc727f@archlinux>

On Sun, May 31, 2020 at 03:40:17PM +0100, Jonathan Cameron wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On Mon, 25 May 2020 13:27:44 +0300
> Alexandru Ardelean <alexandru.ardelean@analog.com> wrote:
> 
> > Since changes can come from all sort of places, it may make sense to have
> > this symbol as a dependency to make sure that the 'make allmodconfig' &&
> > 'make allyesconfig' build rules cover this driver as well for a
> > compile-build/test.
> >
> > It seemed useful [recently] when trying to apply a change for this.
> >
> > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
> Makes sense.   Given this sort of change can expose weird an wonderful
> dependencies that were previously hidden, I'll be wanting an
> ack from at91 people.

Agree.

Acked-by: Ludovic Desroches <ludovic.desroches@microchip.com>

Regards

Ludovic

> 
> Jonathan
> 
> > ---
> >  drivers/iio/adc/Kconfig | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> > index c48c00077775..c1f4c0aec265 100644
> > --- a/drivers/iio/adc/Kconfig
> > +++ b/drivers/iio/adc/Kconfig
> > @@ -294,7 +294,7 @@ config ASPEED_ADC
> >
> >  config AT91_ADC
> >       tristate "Atmel AT91 ADC"
> > -     depends on ARCH_AT91
> > +     depends on ARCH_AT91 || COMPILE_TEST
> >       depends on INPUT && SYSFS
> >       select IIO_BUFFER
> >       select IIO_TRIGGERED_BUFFER
> 

_______________________________________________
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 RFCv2 9/9] arm64: Support async page fault
From: Gavin Shan @ 2020-06-02  5:44 UTC (permalink / raw)
  To: Paolo Bonzini, Marc Zyngier
  Cc: catalin.marinas, linux-kernel, shan.gavin, will, kvmarm,
	linux-arm-kernel
In-Reply-To: <5c72c597-732e-7dbf-d056-665674ec1792@redhat.com>

Hi Marc, Paolo,

On 6/1/20 7:21 PM, Paolo Bonzini wrote:
> On 31/05/20 14:44, Marc Zyngier wrote:
>>>
>>> Is there an ARM-approved way to reuse the S2 fault syndromes to detect
>>> async page faults?
>>
>> It would mean being able to set an ESR_EL2 register value into ESR_EL1,
>> and there is nothing in the architecture that would allow that,
> 
> I understand that this is not what you want to do and I'm not proposing
> it, but I want to understand this better: _in practice_ do CPUs check
> closely what is written in ESR_EL1?
> 
> In any case, the only way to implement this, it seems to me, would be a
> completely paravirtualized exception vector that doesn't use ESR at all.
> 
> On the other hand, for the page ready (interrupt) side assigning a PPI
> seems complicated but doable.
> 

Marc suggested to use SDEI in another reply. I think it might be the
appropriate way to deliver page-not-present. To some extent, it could
be regarded as exception, which doesn't use ESR at all. It matches with
what Paolo is thinking of: paravirtualized exception vector that doesn't
use ESR at all. However, it seems it's not supported in kvm-arm yet. So
I assume it needs to be developed from scratch. Marc, could you please
help to confirm? Thanks in advance.

I agree with Paolo PPI (interrupt) might be the best way to deliver
page-ready currently. I don't think SDEI is suitable because there
are no big difference between SDEI and currently used DABT injection
to some extent. With SDEI, We will have the issues we are facing.
For example, some critical code section isn't safe to receive SDEI
if I'm correct.


Thanks,
Gavin

[...]


_______________________________________________
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 v8 0/5] support reserving crashkernel above 4G on arm64 kdump
From: Prabhakar Kushwaha @ 2020-06-02  5:38 UTC (permalink / raw)
  To: John Donnelly
  Cc: Devicetree List, Arnd Bergmann, Baoquan He,
	Linux Doc Mailing List, Chen Zhou, Catalin Marinas,
	Bhupesh Sharma, RuiRui Yang, kexec mailing list,
	Linux Kernel Mailing List, Rob Herring, Simon Horman, James Morse,
	guohanjun, Thomas Gleixner, Prabhakar Kushwaha, Will Deacon,
	Ingo Molnar, linux-arm-kernel
In-Reply-To: <F64A309C-B9C0-45F2-A50D-D677005C33A6@oracle.com>

On Tue, Jun 2, 2020 at 3:29 AM John Donnelly <john.p.donnelly@oracle.com> wrote:
>
> Hi .  See below !
>
> > On Jun 1, 2020, at 4:02 PM, Bhupesh Sharma <bhsharma@redhat.com> wrote:
> >
> > Hi John,
> >
> > On Tue, Jun 2, 2020 at 1:01 AM John Donnelly <John.P.donnelly@oracle.com> wrote:
> >>
> >> Hi,
> >>
> >>
> >> On 6/1/20 7:02 AM, Prabhakar Kushwaha wrote:
> >>> Hi Chen,
> >>>
> >>> On Thu, May 21, 2020 at 3:05 PM Chen Zhou <chenzhou10@huawei.com> wrote:
> >>>> This patch series enable reserving crashkernel above 4G in arm64.
> >>>>
> >>>> There are following issues in arm64 kdump:
> >>>> 1. We use crashkernel=X to reserve crashkernel below 4G, which will fail
> >>>> when there is no enough low memory.
> >>>> 2. Currently, crashkernel=Y@X can be used to reserve crashkernel above 4G,
> >>>> in this case, if swiotlb or DMA buffers are required, crash dump kernel
> >>>> will boot failure because there is no low memory available for allocation.
> >>>>
> >>>> To solve these issues, introduce crashkernel=X,low to reserve specified
> >>>> size low memory.
> >>>> Crashkernel=X tries to reserve memory for the crash dump kernel under
> >>>> 4G. If crashkernel=Y,low is specified simultaneously, reserve spcified
> >>>> size low memory for crash kdump kernel devices firstly and then reserve
> >>>> memory above 4G.
> >>>>
> >>>> When crashkernel is reserved above 4G in memory, that is, crashkernel=X,low
> >>>> is specified simultaneously, kernel should reserve specified size low memory
> >>>> for crash dump kernel devices. So there may be two crash kernel regions, one
> >>>> is below 4G, the other is above 4G.
> >>>> In order to distinct from the high region and make no effect to the use of
> >>>> kexec-tools, rename the low region as "Crash kernel (low)", and add DT property
> >>>> "linux,low-memory-range" to crash dump kernel's dtb to pass the low region.
> >>>>
> >>>> Besides, we need to modify kexec-tools:
> >>>> arm64: kdump: add another DT property to crash dump kernel's dtb(see [1])
> >>>>
> >>>> The previous changes and discussions can be retrieved from:
> >>>>
> >>>> Changes since [v7]
> >>>> - Move x86 CRASH_ALIGN to 2M
> >>>> Suggested by Dave and do some test, move x86 CRASH_ALIGN to 2M.
> >>>> - Update Documentation/devicetree/bindings/chosen.txt
> >>>> Add corresponding documentation to Documentation/devicetree/bindings/chosen.txt suggested by Arnd.
> >>>> - Add Tested-by from Jhon and pk
> >>>>
> >>>> Changes since [v6]
> >>>> - Fix build errors reported by kbuild test robot.
> >>>>
> >>>> Changes since [v5]
> >>>> - Move reserve_crashkernel_low() into kernel/crash_core.c.
> >>>> - Delete crashkernel=X,high.
> >>>> - Modify crashkernel=X,low.
> >>>> If crashkernel=X,low is specified simultaneously, reserve spcified size low
> >>>> memory for crash kdump kernel devices firstly and then reserve memory above 4G.
> >>>> In addition, rename crashk_low_res as "Crash kernel (low)" for arm64, and then
> >>>> pass to crash dump kernel by DT property "linux,low-memory-range".
> >>>> - Update Documentation/admin-guide/kdump/kdump.rst.
> >>>>
> >>>> Changes since [v4]
> >>>> - Reimplement memblock_cap_memory_ranges for multiple ranges by Mike.
> >>>>
> >>>> Changes since [v3]
> >>>> - Add memblock_cap_memory_ranges back for multiple ranges.
> >>>> - Fix some compiling warnings.
> >>>>
> >>>> Changes since [v2]
> >>>> - Split patch "arm64: kdump: support reserving crashkernel above 4G" as
> >>>> two. Put "move reserve_crashkernel_low() into kexec_core.c" in a separate
> >>>> patch.
> >>>>
> >>>> Changes since [v1]:
> >>>> - Move common reserve_crashkernel_low() code into kernel/kexec_core.c.
> >>>> - Remove memblock_cap_memory_ranges() i added in v1 and implement that
> >>>> in fdt_enforce_memory_region().
> >>>> There are at most two crash kernel regions, for two crash kernel regions
> >>>> case, we cap the memory range [min(regs[*].start), max(regs[*].end)]
> >>>> and then remove the memory range in the middle.
> >>>>
> >>>> [1]: https://urldefense.com/v3/__http://lists.infradead.org/pipermail/kexec/2020-May/025128.html__;!!GqivPVa7Brio!LnTSARkCt0V0FozR0KmqooaH5ADtdXvs3mPdP3KRVqALmvSK2VmCkIPIhsaxbvpn1uM1$
> >>>> [v1]: https://urldefense.com/v3/__https://lkml.org/lkml/2019/4/2/1174__;!!GqivPVa7Brio!LnTSARkCt0V0FozR0KmqooaH5ADtdXvs3mPdP3KRVqALmvSK2VmCkIPIhsaxbt0xN9PE$
> >>>> [v2]: https://urldefense.com/v3/__https://lkml.org/lkml/2019/4/9/86__;!!GqivPVa7Brio!LnTSARkCt0V0FozR0KmqooaH5ADtdXvs3mPdP3KRVqALmvSK2VmCkIPIhsaxbub7yUQH$
> >>>> [v3]: https://urldefense.com/v3/__https://lkml.org/lkml/2019/4/9/306__;!!GqivPVa7Brio!LnTSARkCt0V0FozR0KmqooaH5ADtdXvs3mPdP3KRVqALmvSK2VmCkIPIhsaxbnc4zPPV$
> >>>> [v4]: https://urldefense.com/v3/__https://lkml.org/lkml/2019/4/15/273__;!!GqivPVa7Brio!LnTSARkCt0V0FozR0KmqooaH5ADtdXvs3mPdP3KRVqALmvSK2VmCkIPIhsaxbvsAsZLu$
> >>>> [v5]: https://urldefense.com/v3/__https://lkml.org/lkml/2019/5/6/1360__;!!GqivPVa7Brio!LnTSARkCt0V0FozR0KmqooaH5ADtdXvs3mPdP3KRVqALmvSK2VmCkIPIhsaxbl24n-79$
> >>>> [v6]: https://urldefense.com/v3/__https://lkml.org/lkml/2019/8/30/142__;!!GqivPVa7Brio!LnTSARkCt0V0FozR0KmqooaH5ADtdXvs3mPdP3KRVqALmvSK2VmCkIPIhsaxbs7r8G2a$
> >>>> [v7]: https://urldefense.com/v3/__https://lkml.org/lkml/2019/12/23/411__;!!GqivPVa7Brio!LnTSARkCt0V0FozR0KmqooaH5ADtdXvs3mPdP3KRVqALmvSK2VmCkIPIhsaxbiFUH90G$
> >>>>
> >>>> Chen Zhou (5):
> >>>>   x86: kdump: move reserve_crashkernel_low() into crash_core.c
> >>>>   arm64: kdump: reserve crashkenel above 4G for crash dump kernel
> >>>>   arm64: kdump: add memory for devices by DT property, low-memory-range
> >>>>   kdump: update Documentation about crashkernel on arm64
> >>>>   dt-bindings: chosen: Document linux,low-memory-range for arm64 kdump
> >>>>
> >>> We are getting "warn_alloc" [1] warning during boot of kdump kernel
> >>> with bootargs as [2] of primary kernel.
> >>> This error observed on ThunderX2  ARM64 platform.
> >>>
> >>> It is observed with latest upstream tag (v5.7-rc3) with this patch set
> >>>  and https://urldefense.com/v3/__https://lists.infradead.org/pipermail/kexec/2020-May/025128.html__;!!GqivPVa7Brio!LnTSARkCt0V0FozR0KmqooaH5ADtdXvs3mPdP3KRVqALmvSK2VmCkIPIhsaxbiIAAlzu$
> >>> Also **without** this patch-set
> >>> "https://urldefense.com/v3/__https://www.spinics.net/lists/arm-kernel/msg806882.html__;!!GqivPVa7Brio!LnTSARkCt0V0FozR0KmqooaH5ADtdXvs3mPdP3KRVqALmvSK2VmCkIPIhsaxbjC6ujMA$"
> >>>
> >>> This issue comes whenever crashkernel memory is reserved after 0xc000_0000.
> >>> More details discussed earlier in
> >>> https://urldefense.com/v3/__https://www.spinics.net/lists/arm-kernel/msg806882.html__;!!GqivPVa7Brio!LnTSARkCt0V0FozR0KmqooaH5ADtdXvs3mPdP3KRVqALmvSK2VmCkIPIhsaxbjC6ujMA$  without any
> >>> solution
> >>>
> >>> This patch-set is expected to solve similar kind of issue.
> >>> i.e. low memory is only targeted for DMA, swiotlb; So above mentioned
> >>> observation should be considered/fixed. .
> >>>
> >>> --pk
> >>>
> >>> [1]
> >>> [   30.366695] DMI: Cavium Inc. Saber/Saber, BIOS
> >>> TX2-FW-Release-3.1-build_01-2803-g74253a541a mm/dd/yyyy
> >>> [   30.367696] NET: Registered protocol family 16
> >>> [   30.369973] swapper/0: page allocation failure: order:6,
> >>> mode:0x1(GFP_DMA), nodemask=(null),cpuset=/,mems_allowed=0
> >>> [   30.369980] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.7.0-rc3+ #121
> >>> [   30.369981] Hardware name: Cavium Inc. Saber/Saber, BIOS
> >>> TX2-FW-Release-3.1-build_01-2803-g74253a541a mm/dd/yyyy
> >>> [   30.369984] Call trace:
> >>> [   30.369989]  dump_backtrace+0x0/0x1f8
> >>> [   30.369991]  show_stack+0x20/0x30
> >>> [   30.369997]  dump_stack+0xc0/0x10c
> >>> [   30.370001]  warn_alloc+0x10c/0x178
> >>> [   30.370004]  __alloc_pages_slowpath.constprop.111+0xb10/0xb50
> >>> [   30.370006]  __alloc_pages_nodemask+0x2b4/0x300
> >>> [   30.370008]  alloc_page_interleave+0x24/0x98
> >>> [   30.370011]  alloc_pages_current+0xe4/0x108
> >>> [   30.370017]  dma_atomic_pool_init+0x44/0x1a4
> >>> [   30.370020]  do_one_initcall+0x54/0x228
> >>> [   30.370027]  kernel_init_freeable+0x228/0x2cc
> >>> [   30.370031]  kernel_init+0x1c/0x110
> >>> [   30.370034]  ret_from_fork+0x10/0x18
> >>> [   30.370036] Mem-Info:
> >>> [   30.370064] active_anon:0 inactive_anon:0 isolated_anon:0
> >>> [   30.370064]  active_file:0 inactive_file:0 isolated_file:0
> >>> [   30.370064]  unevictable:0 dirty:0 writeback:0 unstable:0
> >>> [   30.370064]  slab_reclaimable:34 slab_unreclaimable:4438
> >>> [   30.370064]  mapped:0 shmem:0 pagetables:14 bounce:0
> >>> [   30.370064]  free:1537719 free_pcp:219 free_cma:0
> >>> [   30.370070] Node 0 active_anon:0kB inactive_anon:0kB
> >>> active_file:0kB inactive_file:0kB unevictable:0kB isolated(anon):0kB
> >>> isolated(file):0kB mapped:0kB dirty:0kB writeback:0kB shmem:0kB
> >>> shmem_thp: 0kB shmem_pmdmapped: 0kB anon_thp: 0kB writeback_tmp:0kB
> >>> unstable:0kB all_unreclaimable? no
> >>> [   30.370073] Node 1 active_anon:0kB inactive_anon:0kB
> >>> active_file:0kB inactive_file:0kB unevictable:0kB isolated(anon):0kB
> >>> isolated(file):0kB mapped:0kB dirty:0kB writeback:0kB shmem:0kB
> >>> shmem_thp: 0kB shmem_pmdmapped: 0kB anon_thp: 0kB writeback_tmp:0kB
> >>> unstable:0kB all_unreclaimable? no
> >>> [   30.370079] Node 0 DMA free:0kB min:0kB low:0kB high:0kB
> >>> reserved_highatomic:0KB active_anon:0kB inactive_anon:0kB
> >>> active_file:0kB inactive_file:0kB unevictable:0kB writepending:0kB
> >>> present:128kB managed:0kB mlocked:0kB kernel_stack:0kB pagetables:0kB
> >>> bounce:0kB free_pcp:0kB local_pcp:0kB free_cma:0kB
> >>> [   30.370084] lowmem_reserve[]: 0 250 6063 6063
> >>> [   30.370090] Node 0 DMA32 free:256000kB min:408kB low:664kB
> >>> high:920kB reserved_highatomic:0KB active_anon:0kB inactive_anon:0kB
> >>> active_file:0kB inactive_file:0kB unevictable:0kB writepending:0kB
> >>> present:269700kB managed:256000kB mlocked:0kB kernel_stack:0kB
> >>> pagetables:0kB bounce:0kB free_pcp:0kB local_pcp:0kB free_cma:0kB
> >>> [   30.370094] lowmem_reserve[]: 0 0 5813 5813
> >>> [   30.370100] Node 0 Normal free:5894876kB min:9552kB low:15504kB
> >>> high:21456kB reserved_highatomic:0KB active_anon:0kB inactive_anon:0kB
> >>> active_file:0kB inactive_file:0kB unevictable:0kB writepending:0kB
> >>> present:8388608kB managed:5953112kB mlocked:0kB kernel_stack:21672kB
> >>> pagetables:56kB bounce:0kB free_pcp:876kB local_pcp:176kB free_cma:0kB
> >>> [   30.370104] lowmem_reserve[]: 0 0 0 0
> >>> [   30.370107] Node 0 DMA: 0*4kB 0*8kB 0*16kB 0*32kB 0*64kB 0*128kB
> >>> 0*256kB 0*512kB 0*1024kB 0*2048kB 0*4096kB = 0kB
> >>> [   30.370113] Node 0 DMA32: 0*4kB 0*8kB 0*16kB 0*32kB 0*64kB 0*128kB
> >>> 0*256kB 0*512kB 0*1024kB 1*2048kB (M) 62*4096kB (M) = 256000kB
> >>> [   30.370119] Node 0 Normal: 2*4kB (M) 3*8kB (ME) 2*16kB (UE) 3*32kB
> >>> (UM) 1*64kB (U) 2*128kB (M) 2*256kB (ME) 3*512kB (ME) 3*1024kB (ME)
> >>> 3*2048kB (UME) 1436*4096kB (M) = 5893600kB
> >>> [   30.370129] Node 0 hugepages_total=0 hugepages_free=0
> >>> hugepages_surp=0 hugepages_size=1048576kB
> >>> [   30.370130] 0 total pagecache pages
> >>> [   30.370132] 0 pages in swap cache
> >>> [   30.370134] Swap cache stats: add 0, delete 0, find 0/0
> >>> [   30.370135] Free swap  = 0kB
> >>> [   30.370136] Total swap = 0kB
> >>> [   30.370137] 2164609 pages RAM
> >>> [   30.370139] 0 pages HighMem/MovableOnly
> >>> [   30.370140] 612331 pages reserved
> >>> [   30.370141] 0 pages hwpoisoned
> >>> [   30.370143] DMA: failed to allocate 256 KiB pool for atomic
> >>> coherent allocation
> >>
> >>
> >> During my testing I saw the same error and Chen's  solution corrected it .
> >
> > Which combination you are using on your side? I am using Prabhakar's
> > suggested environment and can reproduce the issue
> > with or without Chen's crashkernel support above 4G patchset.
> >
> > I am also using a ThunderX2 platform with latest makedumpfile code and
> > kexec-tools (with the suggested patch
> > <https://urldefense.com/v3/__https://lists.infradead.org/pipermail/kexec/2020-May/025128.html__;!!GqivPVa7Brio!J6lUig58-Gw6TKZnEEYzEeSU36T-1SqlB1kImU00xtX_lss5Tx-JbUmLE9TJC3foXBLg$ >).
> >
> > Thanks,
> > Bhupesh
>
>
> I did this activity 5 months ago and I have moved on to other activities. My DMA failures were related to PCI devices that could not be enumerated because  low-DMA space was not  available  when crashkernel was moved above 4G; I don’t recall the exact platform.
>
>
>
> For this failure ,
>
> >>>  DMA: failed to allocate 256 KiB pool for atomic
> >>> coherent allocation
>
>
> Is due to :
>
>
>  3618082c
>  ("arm64 use both ZONE_DMA and ZONE_DMA32")
>
> With the introduction of ZONE_DMA to support the Raspberry DMA
> region below 1G, the crashkernel is placed in the upper 4G
> ZONE_DMA_32 region. Since the crashkernel does not have access
> to the ZONE_DMA region, it prints out call trace during bootup.
>
> It is due to having this CONFIG item  ON  :
>
>
> CONFIG_ZONE_DMA=y
>
> Turning off ZONE_DMA fixes a issue and Raspberry PI 4 will
> use the device tree to specify memory below 1G.
>
>

Disabling ZONE_DMA is temporary solution.  We may need proper solution

> I would like to see Chen’s feature added , perhaps as EXPERIMENTAL,  so we can get some configuration testing done on it.   It corrects having a DMA zone in low memory while crash-kernel is above 4GB.  This has been going on for a year now.

I will also like this patch to be added in Linux as early as possible.

Issue mentioned by me happens with or without this patch.

This patch-set can consider fixing because it uses low memory for DMA
& swiotlb only.
We can consider restricting crashkernel within the required range like below

diff --git a/kernel/crash_core.c b/kernel/crash_core.c
index 7f9e5a6dc48c..bd67b90d35bd 100644
--- a/kernel/crash_core.c
+++ b/kernel/crash_core.c
@@ -354,7 +354,7 @@ int __init reserve_crashkernel_low(void)
                        return 0;
        }

-       low_base = memblock_find_in_range(0, 1ULL << 32, low_size, CRASH_ALIGN);
+       low_base = memblock_find_in_range(0, 0xc0000000, low_size, CRASH_ALIGN);
        if (!low_base) {
                pr_err("Cannot reserve %ldMB crashkernel low memory,
please try smaller size.\n",
                       (unsigned long)(low_size >> 20));


Similar change can be considered for scenario "without" this patch.
But it will decrease memory availability for crashkernel.
Hence increase the failure probability of crashkernel reservation.

--pk

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [PATCH v2] driver core: platform: expose numa_node to users in sysfs
From: Barry Song @ 2020-06-02  5:36 UTC (permalink / raw)
  To: gregkh, rafael
  Cc: Barry Song, linux-kernel, linuxarm, iommu, Prime Zeng,
	Robin Murphy, linux-arm-kernel

For some platform devices like iommu, particually ARM smmu, users may
care about the numa locality. for example, if threads and drivers run
near iommu, they may get much better performance on dma_unmap_sg.
For other platform devices, users may still want to know the hardware
topology.

Cc: Prime Zeng <prime.zeng@hisilicon.com>
Cc: Robin Murphy <robin.murphy@arm.com>
Signed-off-by: Barry Song <song.bao.hua@hisilicon.com>
---
 -v2: add the numa_node entry in Documentation/ABI/

 Documentation/ABI/testing/sysfs-bus-platform | 10 ++++++++
 drivers/base/platform.c                      | 26 +++++++++++++++++++-
 2 files changed, 35 insertions(+), 1 deletion(-)

diff --git a/Documentation/ABI/testing/sysfs-bus-platform b/Documentation/ABI/testing/sysfs-bus-platform
index 5172a6124b27..e8f5958c1d18 100644
--- a/Documentation/ABI/testing/sysfs-bus-platform
+++ b/Documentation/ABI/testing/sysfs-bus-platform
@@ -18,3 +18,13 @@ Description:
 		devices to opt-out of driver binding using a driver_override
 		name such as "none".  Only a single driver may be specified in
 		the override, there is no support for parsing delimiters.
+
+What:		/sys/bus/platform/devices/.../numa_node
+Date:		June 2020
+Contact:	Barry Song <song.bao.hua@hisilicon.com>
+Description:
+		This file contains the NUMA node to which the platform device
+		is attached. It won't be visible if the node is unknown. The
+		value comes from an ACPI _PXM method or a similar firmware
+		source. Initial users for this file would be devices like
+		arm smmu which are populated by arm64 acpi_iort.
diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index b27d0f6c18c9..7794b9a38d82 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -1062,13 +1062,37 @@ static ssize_t driver_override_show(struct device *dev,
 }
 static DEVICE_ATTR_RW(driver_override);
 
+static ssize_t numa_node_show(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	return sprintf(buf, "%d\n", dev_to_node(dev));
+}
+static DEVICE_ATTR_RO(numa_node);
+
+static umode_t platform_dev_attrs_visible(struct kobject *kobj, struct attribute *a,
+		int n)
+{
+	struct device *dev = container_of(kobj, typeof(*dev), kobj);
+
+	if (a == &dev_attr_numa_node.attr &&
+			dev_to_node(dev) == NUMA_NO_NODE)
+		return 0;
+
+	return a->mode;
+}
 
 static struct attribute *platform_dev_attrs[] = {
 	&dev_attr_modalias.attr,
+	&dev_attr_numa_node.attr,
 	&dev_attr_driver_override.attr,
 	NULL,
 };
-ATTRIBUTE_GROUPS(platform_dev);
+
+static struct attribute_group platform_dev_group = {
+	.attrs = platform_dev_attrs,
+	.is_visible = platform_dev_attrs_visible,
+};
+__ATTRIBUTE_GROUPS(platform_dev);
 
 static int platform_uevent(struct device *dev, struct kobj_uevent_env *env)
 {
-- 
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 v3 1/1] clk: rockchip: rk3288: Handle clock tree for rk3288w
From: Mylene Josserand @ 2020-06-02  5:34 UTC (permalink / raw)
  To: Heiko Stübner
  Cc: sboyd, mturquette, linux-kernel, kever.yang, linux-rockchip,
	geert, kernel, linux-clk, linux-arm-kernel
In-Reply-To: <8288442.SvGebX2C5V@diego>

Hi Heiko,

Thank you very much for your quick review!

On 6/1/20 10:09 PM, Heiko Stübner wrote:
> Hi Mylène,
> 
> Am Montag, 1. Juni 2020, 17:14:42 CEST schrieb Mylène Josserand:
>> The revision rk3288w has a different clock tree about "hclk_vio"
>> clock, according to the BSP kernel code.
>>
>> This patch handles this difference by detecting which device-tree
>> we are using. If it is a "rockchip,rk3288-cru", let's register
>> the clock tree as it was before. If the compatible is
>> "rockchip,rk3288w-cru", we will apply the difference according to this
>> version of this SoC.
>>
>> Noticed that this new device-tree compatible must be handled by
>> bootloader.
>>
>> Signed-off-by: Mylène Josserand <mylene.josserand@collabora.com>
> 
> approach looks good, but you should also update the clock-controller
> dt-binding for the new compatible.

Okay, I will. As it was not implemented in the Kernel, I didn't know if 
I should add it.

> 
> Style nits below.
> 
> 
>> ---
>>   drivers/clk/rockchip/clk-rk3288.c | 20 ++++++++++++++++++--
>>   1 file changed, 18 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/clk/rockchip/clk-rk3288.c b/drivers/clk/rockchip/clk-rk3288.c
>> index cc2a177bbdbf..5018d2f1e54c 100644
>> --- a/drivers/clk/rockchip/clk-rk3288.c
>> +++ b/drivers/clk/rockchip/clk-rk3288.c
>> @@ -425,8 +425,6 @@ static struct rockchip_clk_branch rk3288_clk_branches[] __initdata = {
>>   	COMPOSITE(0, "aclk_vio0", mux_pll_src_cpll_gpll_usb480m_p, CLK_IGNORE_UNUSED,
>>   			RK3288_CLKSEL_CON(31), 6, 2, MFLAGS, 0, 5, DFLAGS,
>>   			RK3288_CLKGATE_CON(3), 0, GFLAGS),
>> -	DIV(0, "hclk_vio", "aclk_vio0", 0,
>> -			RK3288_CLKSEL_CON(28), 8, 5, DFLAGS),
>>   	COMPOSITE(0, "aclk_vio1", mux_pll_src_cpll_gpll_usb480m_p, CLK_IGNORE_UNUSED,
>>   			RK3288_CLKSEL_CON(31), 14, 2, MFLAGS, 8, 5, DFLAGS,
>>   			RK3288_CLKGATE_CON(3), 2, GFLAGS),
>> @@ -819,6 +817,16 @@ static struct rockchip_clk_branch rk3288_clk_branches[] __initdata = {
>>   	INVERTER(0, "pclk_isp", "pclk_isp_in", RK3288_CLKSEL_CON(29), 3, IFLAGS),
>>   };
>>   
>> +static struct rockchip_clk_branch rk3288w_hclkvio_branch[] __initdata = {
>> +	DIV(0, "hclk_vio", "aclk_vio1", 0,
>> +	    RK3288_CLKSEL_CON(28), 8, 5, DFLAGS),
> 
> please keep indentations as they were, the sub-lines starting where they
> are is actually intentional :-)

Oups, I didn't know, I will update this in my V4.

> 
> 
>> +};
>> +
>> +static struct rockchip_clk_branch rk3288_hclkvio_branch[] __initdata = {
>> +	DIV(0, "hclk_vio", "aclk_vio0", 0,
>> +	    RK3288_CLKSEL_CON(28), 8, 5, DFLAGS),
> 
> same here

same here

> 
>> +};
>> +
>>   static const char *const rk3288_critical_clocks[] __initconst = {
>>   	"aclk_cpu",
>>   	"aclk_peri",
>> @@ -936,6 +944,14 @@ static void __init rk3288_clk_init(struct device_node *np)
>>   				   RK3288_GRF_SOC_STATUS1);
>>   	rockchip_clk_register_branches(ctx, rk3288_clk_branches,
>>   				  ARRAY_SIZE(rk3288_clk_branches));
>> +
>> +	if (of_device_is_compatible(np, "rockchip,rk3288w-cru"))
>> +		rockchip_clk_register_branches(ctx, rk3288w_hclkvio_branch,
>> +					       ARRAY_SIZE(rk3288w_hclkvio_branch));
>> +	else
>> +		rockchip_clk_register_branches(ctx, rk3288_hclkvio_branch,
>> +					       ARRAY_SIZE(rk3288_hclkvio_branch));
>> +
>>   	rockchip_clk_protect_critical(rk3288_critical_clocks,
>>   				      ARRAY_SIZE(rk3288_critical_clocks));
>>   
>>

Best regards,
Mylène

_______________________________________________
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 V3 1/3] dt-bindings: mailbox: imx-mu: support i.MX8M
From: Peng Fan @ 2020-06-02  5:26 UTC (permalink / raw)
  To: Oleksij Rempel
  Cc: Aisheng Dong, devicetree@vger.kernel.org, sboyd@kernel.org,
	Daniel Baluta, linux-kernel@vger.kernel.org,
	linux@rempel-privat.de, jaswinder.singh@linaro.org,
	robh+dt@kernel.org, dl-linux-imx, kernel@pengutronix.de,
	Fabio Estevam, Leonard Crestez, shawnguo@kernel.org,
	linux-clk@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	l.stach@pengutronix.de
In-Reply-To: <20200602052448.fxepmwltc4465q4i@pengutronix.de>

Hi Oleksij,

> Subject: Re: [PATCH V3 1/3] dt-bindings: mailbox: imx-mu: support i.MX8M
> 
> On Mon, Jun 01, 2020 at 04:20:00PM +0800, peng.fan@nxp.com wrote:
> > From: Peng Fan <peng.fan@nxp.com>
> >
> > Add i.MX8MQ/M/N/P compatible string to support i.MX8M SoCs
> >
> > Reviewed-by: Dong Aisheng <aisheng.dong@nxp.com>
> > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > ---
> >  Documentation/devicetree/bindings/mailbox/fsl,mu.txt | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/devicetree/bindings/mailbox/fsl,mu.txt
> > b/Documentation/devicetree/bindings/mailbox/fsl,mu.txt
> > index 26b7a88c2fea..906377acf2cd 100644
> > --- a/Documentation/devicetree/bindings/mailbox/fsl,mu.txt
> > +++ b/Documentation/devicetree/bindings/mailbox/fsl,mu.txt
> > @@ -18,7 +18,8 @@ Messaging Unit Device Node:
> >  Required properties:
> >  -------------------
> >  - compatible :	should be "fsl,<chip>-mu", the supported chips include
> > -		imx6sx, imx7s, imx8qxp, imx8qm.
> > +		imx6sx, imx7s, imx8qxp, imx8qm, imx8mq, imx8mm, imx8mn,
> > +		imx8mp.
> >  		The "fsl,imx6sx-mu" compatible is seen as generic and should
> >  		be included together with SoC specific compatible.
> >  		There is a version 1.0 MU on imx7ulp, use "fsl,imx7ulp-mu"
> > --
> > 2.16.4
> >
> >
> 
> Hi Peng,
> 
> The fsl,mu.yaml was already taken by Rob, so one or other patch will break by
> merge. I assume you should drop this change.

Yes. I'll rebase this patch based on Rob's tree. Thanks for reminding me.

Thanks,
Peng.

> 
> 
> Regards,
> Oleksij
> --
> Pengutronix e.K.                           |
> |
> Steuerwalder Str. 21                       |
> http://www.pengutronix.de/  |
> 31137 Hildesheim, Germany                  | Phone:
> +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:
> +49-5121-206917-5555 |
_______________________________________________
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/2] Append new variables to vmcoreinfo (TCR_EL1.T1SZ for arm64 and MAX_PHYSMEM_BITS for all archs)
From: Bhupesh Sharma @ 2020-06-02  5:24 UTC (permalink / raw)
  To: linux-arm-kernel, x86
  Cc: Mark Rutland, Linux Doc Mailing List, Benjamin Herrenschmidt,
	Paul Mackerras, Amit Kachhap, Will Deacon, Ingo Molnar,
	Jonathan Corbet, Michael Ellerman, Catalin Marinas, John Donnelly,
	scott.branden, Boris Petkov, Thomas Gleixner, Bhupesh SHARMA,
	Kazuhito Hagio, Ard Biesheuvel, Steve Capper, kexec mailing list,
	Linux Kernel Mailing List, James Morse, Dave Anderson,
	linuxppc-dev
In-Reply-To: <1589395957-24628-1-git-send-email-bhsharma@redhat.com>

Hello,

On Thu, May 14, 2020 at 12:22 AM Bhupesh Sharma <bhsharma@redhat.com> wrote:
>
> Apologies for the delayed update. Its been quite some time since I
> posted the last version (v5), but I have been really caught up in some
> other critical issues.
>
> Changes since v5:
> ----------------
> - v5 can be viewed here:
>   http://lists.infradead.org/pipermail/kexec/2019-November/024055.html
> - Addressed review comments from James Morse and Boris.
> - Added Tested-by received from John on v5 patchset.
> - Rebased against arm64 (for-next/ptr-auth) branch which has Amit's
>   patchset for ARMv8.3-A Pointer Authentication feature vmcoreinfo
>   applied.
>
> Changes since v4:
> ----------------
> - v4 can be seen here:
>   http://lists.infradead.org/pipermail/kexec/2019-November/023961.html
> - Addressed comments from Dave and added patches for documenting
>   new variables appended to vmcoreinfo documentation.
> - Added testing report shared by Akashi for PATCH 2/5.
>
> Changes since v3:
> ----------------
> - v3 can be seen here:
>   http://lists.infradead.org/pipermail/kexec/2019-March/022590.html
> - Addressed comments from James and exported TCR_EL1.T1SZ in vmcoreinfo
>   instead of PTRS_PER_PGD.
> - Added a new patch (via [PATCH 3/3]), which fixes a simple typo in
>   'Documentation/arm64/memory.rst'
>
> Changes since v2:
> ----------------
> - v2 can be seen here:
>   http://lists.infradead.org/pipermail/kexec/2019-March/022531.html
> - Protected 'MAX_PHYSMEM_BITS' vmcoreinfo variable under CONFIG_SPARSEMEM
>   ifdef sections, as suggested by Kazu.
> - Updated vmcoreinfo documentation to add description about
>   'MAX_PHYSMEM_BITS' variable (via [PATCH 3/3]).
>
> Changes since v1:
> ----------------
> - v1 was sent out as a single patch which can be seen here:
>   http://lists.infradead.org/pipermail/kexec/2019-February/022411.html
>
> - v2 breaks the single patch into two independent patches:
>   [PATCH 1/2] appends 'PTRS_PER_PGD' to vmcoreinfo for arm64 arch, whereas
>   [PATCH 2/2] appends 'MAX_PHYSMEM_BITS' to vmcoreinfo in core kernel code (all archs)
>
> This patchset primarily fixes the regression reported in user-space
> utilities like 'makedumpfile' and 'crash-utility' on arm64 architecture
> with the availability of 52-bit address space feature in underlying
> kernel. These regressions have been reported both on CPUs which don't
> support ARMv8.2 extensions (i.e. LVA, LPA) and are running newer kernels
> and also on prototype platforms (like ARMv8 FVP simulator model) which
> support ARMv8.2 extensions and are running newer kernels.
>
> The reason for these regressions is that right now user-space tools
> have no direct access to these values (since these are not exported
> from the kernel) and hence need to rely on a best-guess method of
> determining value of 'vabits_actual' and 'MAX_PHYSMEM_BITS' supported
> by underlying kernel.
>
> Exporting these values via vmcoreinfo will help user-land in such cases.
> In addition, as per suggestion from makedumpfile maintainer (Kazu),
> it makes more sense to append 'MAX_PHYSMEM_BITS' to
> vmcoreinfo in the core code itself rather than in arm64 arch-specific
> code, so that the user-space code for other archs can also benefit from
> this addition to the vmcoreinfo and use it as a standard way of
> determining 'SECTIONS_SHIFT' value in user-land.
>
> Cc: Boris Petkov <bp@alien8.de>
> Cc: Ingo Molnar <mingo@kernel.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Jonathan Corbet <corbet@lwn.net>
> Cc: James Morse <james.morse@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: Steve Capper <steve.capper@arm.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Dave Anderson <anderson@redhat.com>
> Cc: Kazuhito Hagio <k-hagio@ab.jp.nec.com>
> Cc: John Donnelly <john.p.donnelly@oracle.com>
> Cc: scott.branden@broadcom.com
> Cc: Amit Kachhap <amit.kachhap@arm.com>
> Cc: x86@kernel.org
> Cc: linuxppc-dev@lists.ozlabs.org
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-kernel@vger.kernel.org
> Cc: linux-doc@vger.kernel.org
> Cc: kexec@lists.infradead.org
>
> Bhupesh Sharma (2):
>   crash_core, vmcoreinfo: Append 'MAX_PHYSMEM_BITS' to vmcoreinfo
>   arm64/crash_core: Export TCR_EL1.T1SZ in vmcoreinfo
>
>  Documentation/admin-guide/kdump/vmcoreinfo.rst | 16 ++++++++++++++++
>  arch/arm64/include/asm/pgtable-hwdef.h         |  1 +
>  arch/arm64/kernel/crash_core.c                 | 10 ++++++++++
>  kernel/crash_core.c                            |  1 +
>  4 files changed, 28 insertions(+)

Ping. @James Morse , Others

Please share if you have some comments regarding this patchset.

Thanks,
Bhupesh


_______________________________________________
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 V3 1/3] dt-bindings: mailbox: imx-mu: support i.MX8M
From: Oleksij Rempel @ 2020-06-02  5:24 UTC (permalink / raw)
  To: peng.fan
  Cc: aisheng.dong, devicetree, sboyd, daniel.baluta, linux-kernel,
	linux, jaswinder.singh, robh+dt, linux-imx, kernel, fabio.estevam,
	leonard.crestez, shawnguo, linux-clk, linux-arm-kernel, l.stach
In-Reply-To: <1590999602-29482-2-git-send-email-peng.fan@nxp.com>


[-- Attachment #1.1: Type: text/plain, Size: 1661 bytes --]

On Mon, Jun 01, 2020 at 04:20:00PM +0800, peng.fan@nxp.com wrote:
> From: Peng Fan <peng.fan@nxp.com>
> 
> Add i.MX8MQ/M/N/P compatible string to support i.MX8M SoCs
> 
> Reviewed-by: Dong Aisheng <aisheng.dong@nxp.com>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
>  Documentation/devicetree/bindings/mailbox/fsl,mu.txt | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/mailbox/fsl,mu.txt b/Documentation/devicetree/bindings/mailbox/fsl,mu.txt
> index 26b7a88c2fea..906377acf2cd 100644
> --- a/Documentation/devicetree/bindings/mailbox/fsl,mu.txt
> +++ b/Documentation/devicetree/bindings/mailbox/fsl,mu.txt
> @@ -18,7 +18,8 @@ Messaging Unit Device Node:
>  Required properties:
>  -------------------
>  - compatible :	should be "fsl,<chip>-mu", the supported chips include
> -		imx6sx, imx7s, imx8qxp, imx8qm.
> +		imx6sx, imx7s, imx8qxp, imx8qm, imx8mq, imx8mm, imx8mn,
> +		imx8mp.
>  		The "fsl,imx6sx-mu" compatible is seen as generic and should
>  		be included together with SoC specific compatible.
>  		There is a version 1.0 MU on imx7ulp, use "fsl,imx7ulp-mu"
> -- 
> 2.16.4
> 
> 

Hi Peng,

The fsl,mu.yaml was already taken by Rob, so one or other patch will
break by merge. I assume you should drop this change.


Regards,
Oleksij
-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox