* [rjarzmik:work/dma_slave_map 5/13] drivers/mtd/nand/raw/marvell_nand.c:2628:52: sparse: incorrect type in argument 1 (different base types)
From: kbuild test robot @ 2018-05-24 9:07 UTC (permalink / raw)
To: linux-arm-kernel
tree: https://github.com/rjarzmik/linux work/dma_slave_map
head: d18138ed977685f67fb18efcb5b5ef7084e3f1ae
commit: 38686a4ca1306465bae33d821a1128288f68ec6b [5/13] mtd: rawnand: marvell: remove the dmaengine compat need
reproduce:
# apt-get install sparse
git checkout 38686a4ca1306465bae33d821a1128288f68ec6b
make ARCH=x86_64 allmodconfig
make C=1 CF=-D__CHECK_ENDIAN__
sparse warnings: (new ones prefixed by >>)
drivers/mtd/nand/raw/marvell_nand.c:752:32: sparse: expression using sizeof(void)
drivers/mtd/nand/raw/marvell_nand.c:861:25: sparse: expression using sizeof(void)
drivers/mtd/nand/raw/marvell_nand.c:861:25: sparse: expression using sizeof(void)
drivers/mtd/nand/raw/marvell_nand.c:907:25: sparse: expression using sizeof(void)
drivers/mtd/nand/raw/marvell_nand.c:907:25: sparse: expression using sizeof(void)
drivers/mtd/nand/raw/marvell_nand.c:1572:41: sparse: expression using sizeof(void)
drivers/mtd/nand/raw/marvell_nand.c:2238:24: sparse: expression using sizeof(void)
drivers/mtd/nand/raw/marvell_nand.c:2238:24: sparse: expression using sizeof(void)
drivers/mtd/nand/raw/marvell_nand.c:2240:24: sparse: expression using sizeof(void)
drivers/mtd/nand/raw/marvell_nand.c:2240:24: sparse: expression using sizeof(void)
drivers/mtd/nand/raw/marvell_nand.c:2262:17: sparse: expression using sizeof(void)
drivers/mtd/nand/raw/marvell_nand.c:2263:17: sparse: expression using sizeof(void)
drivers/mtd/nand/raw/marvell_nand.c:2264:17: sparse: expression using sizeof(void)
drivers/mtd/nand/raw/marvell_nand.c:2265:17: sparse: expression using sizeof(void)
drivers/mtd/nand/raw/marvell_nand.c:2266:17: sparse: expression using sizeof(void)
drivers/mtd/nand/raw/marvell_nand.c:2267:17: sparse: expression using sizeof(void)
drivers/mtd/nand/raw/marvell_nand.c:2268:17: sparse: expression using sizeof(void)
drivers/mtd/nand/raw/marvell_nand.c:2271:17: sparse: expression using sizeof(void)
drivers/mtd/nand/raw/marvell_nand.c:2272:17: sparse: expression using sizeof(void)
drivers/mtd/nand/raw/marvell_nand.c:2273:17: sparse: expression using sizeof(void)
drivers/mtd/nand/raw/marvell_nand.c:2277:25: sparse: expression using sizeof(void)
drivers/mtd/nand/raw/marvell_nand.c:2279:25: sparse: expression using sizeof(void)
drivers/mtd/nand/raw/marvell_nand.c:2282:25: sparse: expression using sizeof(void)
>> drivers/mtd/nand/raw/marvell_nand.c:2628:52: sparse: incorrect type in argument 1 (different base types) @@ expected struct device *dev @@ got sstruct device *dev @@
drivers/mtd/nand/raw/marvell_nand.c:2628:52: expected struct device *dev
drivers/mtd/nand/raw/marvell_nand.c:2628:52: got struct device **<noident>
drivers/mtd/nand/raw/marvell_nand.c: In function 'marvell_nfc_init_dma':
drivers/mtd/nand/raw/marvell_nand.c:2628:44: error: passing argument 1 of 'dma_request_slave_channel' from incompatible pointer type [-Werror=incompatible-pointer-types]
nfc->dma_chan = dma_request_slave_channel(&nfc->dev, "data");
^
In file included from drivers/mtd/nand/raw/marvell_nand.c:21:0:
include/linux/dmaengine.h:1315:18: note: expected 'struct device *' but argument is of type 'struct device **'
struct dma_chan *dma_request_slave_channel(struct device *dev, const char *name);
^~~~~~~~~~~~~~~~~~~~~~~~~
cc1: some warnings being treated as errors
vim +2628 drivers/mtd/nand/raw/marvell_nand.c
2608
2609 static int marvell_nfc_init_dma(struct marvell_nfc *nfc)
2610 {
2611 struct platform_device *pdev = container_of(nfc->dev,
2612 struct platform_device,
2613 dev);
2614 struct dma_slave_config config = {};
2615 struct resource *r;
2616 int ret;
2617
2618 if (!IS_ENABLED(CONFIG_PXA_DMA)) {
2619 dev_warn(nfc->dev,
2620 "DMA not enabled in configuration\n");
2621 return -ENOTSUPP;
2622 }
2623
2624 ret = dma_set_mask_and_coherent(nfc->dev, DMA_BIT_MASK(32));
2625 if (ret)
2626 return ret;
2627
> 2628 nfc->dma_chan = dma_request_slave_channel(&nfc->dev, "data");
2629 if (!nfc->dma_chan) {
2630 dev_err(nfc->dev,
2631 "Unable to request data DMA channel\n");
2632 return -ENODEV;
2633 }
2634
2635 r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
2636 if (!r)
2637 return -ENXIO;
2638
2639 config.src_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
2640 config.dst_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
2641 config.src_addr = r->start + NDDB;
2642 config.dst_addr = r->start + NDDB;
2643 config.src_maxburst = 32;
2644 config.dst_maxburst = 32;
2645 ret = dmaengine_slave_config(nfc->dma_chan, &config);
2646 if (ret < 0) {
2647 dev_err(nfc->dev, "Failed to configure DMA channel\n");
2648 return ret;
2649 }
2650
2651 /*
2652 * DMA must act on length multiple of 32 and this length may be
2653 * bigger than the destination buffer. Use this buffer instead
2654 * for DMA transfers and then copy the desired amount of data to
2655 * the provided buffer.
2656 */
2657 nfc->dma_buf = kmalloc(MAX_CHUNK_SIZE, GFP_KERNEL | GFP_DMA);
2658 if (!nfc->dma_buf)
2659 return -ENOMEM;
2660
2661 nfc->use_dma = true;
2662
2663 return 0;
2664 }
2665
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
^ permalink raw reply
* [PATCH v10 06/18] arm64: fpsimd: Generalise context saving for non-task contexts
From: Dave Martin @ 2018-05-24 9:03 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <87in7et9gw.fsf@linaro.org>
On Wed, May 23, 2018 at 09:15:11PM +0100, Alex Benn?e wrote:
>
> Dave Martin <Dave.Martin@arm.com> writes:
>
> > In preparation for allowing non-task (i.e., KVM vcpu) FPSIMD
> > contexts to be handled by the fpsimd common code, this patch adapts
> > task_fpsimd_save() to save back the currently loaded context,
> > removing the explicit dependency on current.
> >
> > The relevant storage to write back to in memory is now found by
> > examining the fpsimd_last_state percpu struct.
> >
> > fpsimd_save() does nothing unless TIF_FOREIGN_FPSTATE is clear, and
> > fpsimd_last_state is updated under local_bh_disable() or
> > local_irq_disable() everywhere that TIF_FOREIGN_FPSTATE is cleared:
> > thus, fpsimd_save() will write back to the correct storage for the
> > loaded context.
> >
> > No functional change.
> >
> > Signed-off-by: Dave Martin <Dave.Martin@arm.com>
> > Acked-by: Marc Zyngier <marc.zyngier@arm.com>
> > Acked-by: Catalin Marinas <catalin.marinas@arm.com>
> > ---
> > arch/arm64/kernel/fpsimd.c | 25 +++++++++++++------------
> > 1 file changed, 13 insertions(+), 12 deletions(-)
> >
> > diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c
> > index 9d85373..3aa100a 100644
> > --- a/arch/arm64/kernel/fpsimd.c
> > +++ b/arch/arm64/kernel/fpsimd.c
> > @@ -270,13 +270,15 @@ static void task_fpsimd_load(void)
> > }
> >
> > /*
> > - * Ensure current's FPSIMD/SVE storage in thread_struct is up to date
> > - * with respect to the CPU registers.
> > + * Ensure FPSIMD/SVE storage in memory for the loaded context is up to
> > + * date with respect to the CPU registers.
> > *
> > * Softirqs (and preemption) must be disabled.
> > */
> > -static void task_fpsimd_save(void)
> > +static void fpsimd_save(void)
> > {
> > + struct user_fpsimd_state *st = __this_cpu_read(fpsimd_last_state.st);
> > +
>
> I thought I was missing something but the only write I saw of this was:
>
> __this_cpu_write(fpsimd_last_state.st, NULL);
>
> which implied to me it is possible to have an invalid de-reference. I
> did figure it out eventually as fpsimd_bind_state_to_cpu uses a more
> indirect this_cpu_ptr idiom for tweaking this. I guess a reference to
> fpsimd_bind_[task|state]_to_cpu in the comment would have helped my
> confusion.
How about:
static void fpsimd_save(void)
{
struct user_fpsimd_state *st = __this_cpu_read(fpsimd_last_state.st);
+ /* set by fpsimd_bind_to_cpu() */
WARN_ON(!in_softirq() && !irqs_disabled());
> Reviewed-by: Alex Benn?e <alex.bennee@linaro.org>
Thanks
---Dave
^ permalink raw reply
* [PATCH 2/2] soc: imx: add SCU power domains driver
From: A.s. Dong @ 2018-05-24 9:03 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAPDyKFrZHB2ti75zQav_SSXndorV38TcXMCoRBvDzdE5SS-WhA@mail.gmail.com>
Hi Ulf,
> -----Original Message-----
> From: Ulf Hansson [mailto:ulf.hansson at linaro.org]
> Sent: Thursday, May 24, 2018 5:00 PM
> To: A.s. Dong <aisheng.dong@nxp.com>
> Cc: Linux ARM <linux-arm-kernel@lists.infradead.org>; Dong Aisheng
> <dongas86@gmail.com>; Sascha Hauer <kernel@pengutronix.de>; Shawn
> Guo <shawnguo@kernel.org>; Fabio Estevam <fabio.estevam@nxp.com>;
> dl-linux-imx <linux-imx@nxp.com>; Rafael J. Wysocki <rjw@rjwysocki.net>;
> Kevin Hilman <khilman@kernel.org>; Linux PM <linux-pm@vger.kernel.org>
> Subject: Re: [PATCH 2/2] soc: imx: add SCU power domains driver
>
> On 24 May 2018 at 10:37, A.s. Dong <aisheng.dong@nxp.com> wrote:
> > Hi Ulf,
> >
> > Thanks for the review.
> >
> >> -----Original Message-----
> >> From: Ulf Hansson [mailto:ulf.hansson at linaro.org]
> >> Sent: Wednesday, May 9, 2018 3:16 AM
> >> To: A.s. Dong <aisheng.dong@nxp.com>
> >> Cc: Linux ARM <linux-arm-kernel@lists.infradead.org>; Dong Aisheng
> >> <dongas86@gmail.com>; Sascha Hauer <kernel@pengutronix.de>; Shawn
> Guo
> >> <shawnguo@kernel.org>; Fabio Estevam <fabio.estevam@nxp.com>;
> >> dl-linux-imx <linux-imx@nxp.com>; Rafael J. Wysocki
> >> <rjw@rjwysocki.net>; Kevin Hilman <khilman@kernel.org>; Linux PM
> >> <linux-pm@vger.kernel.org>
> >> Subject: Re: [PATCH 2/2] soc: imx: add SCU power domains driver
> >>
> >> [...]
> >>
> >> > +
> >> > +static int __init imx_sc_init_pm_domains(void) {
> >> > + struct generic_pm_domain *pd;
> >> > + struct device_node *np;
> >> > + sc_err_t sci_err;
> >> > +
> >> > + if (!of_machine_is_compatible("fsl,imx8qxp"))
> >> > + return 0;
> >> > +
> >> > + sci_err = sc_ipc_get_handle(&pm_ipc_handle);
> >> > + if (sci_err != SC_ERR_NONE) {
> >> > + pr_err("imx_sc_pd: can't get sc ipc handle\n");
> >> > + return -ENODEV;
> >> > + }
> >> > +
> >> > + for_each_matching_node(np, imx_sc_pm_domain_of_match) {
> >> > + pd = imx_sc_pm_add_one_domain(np, NULL);
> >> > + if (!IS_ERR(pd))
> >> > + imx_sc_pm_add_subdomains(np, pd);
> >> > + }
> >>
> >> Perhaps using of_genpd_add_subdomain() may help here and possibly
> >> could avoid some open coding!?
> >>
> >
> > Thanks for the suggestion. I thought of it a lot and the result is
> > that I'm not sure If it's quite suitable for i.MX cases. Currently
> > seems there's only one user, Samsung, in kernel using that API which
> > takes two struct of_phandle_args as arguments, parent and child. It looks
> needs special handling in code before using it, e.g.
> > register both parent and child domain first, which is somehow not like
> > i.MX flow of registration. It looks like to me not see much benefits
> > to enforce a big change to switch to it. Or am I missed anything?
>
> It's up to you. The idea was to make the code easier and a bit future proof.
> Perhaps that isn't suitable here.
>
> In any case, if you have a device node and all power-domain providers are
> listed below it, one can call for_each_child_of_node() and search for
> provider nodes hierarchically. I thought that is kind of nice.
>
> Something along the lines of what I done in psci_dt_set_genpd_topology(),
> from a patch I posted a while ago:
> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpat
> chwork.kernel.org%2Fpatch%2F10338399%2F&data=02%7C01%7Caisheng.do
> ng%40nxp.com%7C0ce43cb8f66740a0f95908d5c154c330%7C686ea1d3bc2b4c
> 6fa92cd99c5c301635%7C0%7C1%7C636627492163349262&sdata=sk9U7c5VQ
> WKJN1UMOMUIXZh0nnpiWGX7ildx2Zzo4dc%3D&reserved=0
>
Thanks for the info. Will have a look at it.
> [...]
>
> If you decide to sticking to the existing way, anyway feel free to add:
>
> Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>
>
Thanks
Regards
Dong Aisheng
> Kind regards
> Uffe
^ permalink raw reply
* [PATCH 2/2] soc: imx: add SCU power domains driver
From: Ulf Hansson @ 2018-05-24 9:00 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <AM0PR04MB42114331A1E75B3B90B07E56806A0@AM0PR04MB4211.eurprd04.prod.outlook.com>
On 24 May 2018 at 10:37, A.s. Dong <aisheng.dong@nxp.com> wrote:
> Hi Ulf,
>
> Thanks for the review.
>
>> -----Original Message-----
>> From: Ulf Hansson [mailto:ulf.hansson at linaro.org]
>> Sent: Wednesday, May 9, 2018 3:16 AM
>> To: A.s. Dong <aisheng.dong@nxp.com>
>> Cc: Linux ARM <linux-arm-kernel@lists.infradead.org>; Dong Aisheng
>> <dongas86@gmail.com>; Sascha Hauer <kernel@pengutronix.de>; Shawn
>> Guo <shawnguo@kernel.org>; Fabio Estevam <fabio.estevam@nxp.com>;
>> dl-linux-imx <linux-imx@nxp.com>; Rafael J. Wysocki <rjw@rjwysocki.net>;
>> Kevin Hilman <khilman@kernel.org>; Linux PM <linux-pm@vger.kernel.org>
>> Subject: Re: [PATCH 2/2] soc: imx: add SCU power domains driver
>>
>> [...]
>>
>> > +
>> > +static int __init imx_sc_init_pm_domains(void) {
>> > + struct generic_pm_domain *pd;
>> > + struct device_node *np;
>> > + sc_err_t sci_err;
>> > +
>> > + if (!of_machine_is_compatible("fsl,imx8qxp"))
>> > + return 0;
>> > +
>> > + sci_err = sc_ipc_get_handle(&pm_ipc_handle);
>> > + if (sci_err != SC_ERR_NONE) {
>> > + pr_err("imx_sc_pd: can't get sc ipc handle\n");
>> > + return -ENODEV;
>> > + }
>> > +
>> > + for_each_matching_node(np, imx_sc_pm_domain_of_match) {
>> > + pd = imx_sc_pm_add_one_domain(np, NULL);
>> > + if (!IS_ERR(pd))
>> > + imx_sc_pm_add_subdomains(np, pd);
>> > + }
>>
>> Perhaps using of_genpd_add_subdomain() may help here and possibly could
>> avoid some open coding!?
>>
>
> Thanks for the suggestion. I thought of it a lot and the result is that I'm not sure
> If it's quite suitable for i.MX cases. Currently seems there's only one user, Samsung,
> in kernel using that API which takes two struct of_phandle_args as arguments,
> parent and child. It looks needs special handling in code before using it, e.g.
> register both parent and child domain first, which is somehow not like i.MX flow
> of registration. It looks like to me not see much benefits to enforce a big
> change to switch to it. Or am I missed anything?
It's up to you. The idea was to make the code easier and a bit future
proof. Perhaps that isn't suitable here.
In any case, if you have a device node and all power-domain providers
are listed below it, one can call for_each_child_of_node() and search
for provider nodes hierarchically. I thought that is kind of nice.
Something along the lines of what I done in
psci_dt_set_genpd_topology(), from a patch I posted a while ago:
https://patchwork.kernel.org/patch/10338399/
[...]
If you decide to sticking to the existing way, anyway feel free to add:
Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>
Kind regards
Uffe
^ permalink raw reply
* [PATCH 4/4] soc: imx: add SC firmware IPC and APIs
From: A.s. Dong @ 2018-05-24 8:56 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180503124030.jkc7oox2vyjblk2y@pengutronix.de>
Hi Sascha,
> -----Original Message-----
> From: Sascha Hauer [mailto:s.hauer at pengutronix.de]
> Sent: Thursday, May 3, 2018 8:41 PM
> To: A.s. Dong <aisheng.dong@nxp.com>
> Cc: linux-arm-kernel at lists.infradead.org; dongas86 at gmail.com; dl-linux-imx
> <linux-imx@nxp.com>; kernel at pengutronix.de; Fabio Estevam
> <fabio.estevam@nxp.com>; shawnguo at kernel.org
> Subject: Re: [PATCH 4/4] soc: imx: add SC firmware IPC and APIs
>
> On Thu, May 03, 2018 at 12:29:40PM +0000, A.s. Dong wrote:
> > > -----Original Message-----
> > > From: Sascha Hauer [mailto:s.hauer at pengutronix.de]
> > > Sent: Thursday, May 3, 2018 7:06 PM
> > > To: A.s. Dong <aisheng.dong@nxp.com>
> > > Cc: linux-arm-kernel at lists.infradead.org; dongas86 at gmail.com;
> > > dl-linux-imx <linux-imx@nxp.com>; kernel at pengutronix.de; Fabio
> > > Estevam <fabio.estevam@nxp.com>; shawnguo at kernel.org
> > > Subject: Re: [PATCH 4/4] soc: imx: add SC firmware IPC and APIs
> > >
> > > On Wed, May 02, 2018 at 06:40:03PM +0000, A.s. Dong wrote:
> > > > > -----Original Message-----
> > > > > From: Sascha Hauer [mailto:s.hauer at pengutronix.de]
> > > > > Sent: Wednesday, May 2, 2018 6:04 PM
> > > > > To: A.s. Dong <aisheng.dong@nxp.com>
> > > > > Cc: linux-arm-kernel at lists.infradead.org; dongas86 at gmail.com;
> > > > > dl-linux-imx <linux-imx@nxp.com>; kernel at pengutronix.de; Fabio
> > > > > Estevam <fabio.estevam@nxp.com>; shawnguo at kernel.org
> > > > > Subject: Re: [PATCH 4/4] soc: imx: add SC firmware IPC and APIs
> > > > >
> > > > > On Sat, Apr 28, 2018 at 02:46:16AM +0800, Dong Aisheng wrote:
> > > > > > +/* Initialization of the MU code. */ int __init
> > > > > > +imx8_scu_init(void) {
> > > > > > + struct device_node *np, *mu_np;
> > > > > > + struct resource mu_res;
> > > > > > + sc_err_t sci_err;
> > > > > > + int ret;
> > > > > > +
> > > > > > + if (!of_machine_is_compatible("fsl,imx8qxp"))
> > > > > > + return 0;
> > > > > > +
> > > > > > + np = of_find_compatible_node(NULL, NULL, "nxp,imx8qxp-
> scu");
> > > > > > + if (!np)
> > > > > > + return -ENODEV;
> > > > > > +
> > > > > > + mu_np = of_parse_phandle(np, "fsl,mu", 0);
> > > > > > + if (WARN_ON(!mu_np))
> > > > > > + return -EINVAL;
> > > > > > +
> > > > > > + ret = of_address_to_resource(mu_np, 0, &mu_res);
> > > > > > + if (WARN_ON(ret))
> > > > > > + return -EINVAL;
> > > > > > +
> > > > > > + /* we use mu physical address as IPC communication channel
> ID */
> > > > > > + sci_err = sc_ipc_open(&scu_ipc_handle,
> (sc_ipc_id_t)(mu_res.start));
> > > > > > + if (sci_err != SC_ERR_NONE) {
> > > > > > + pr_err("Cannot open MU channel to SCU\n");
> > > > > > + return sci_err;
> > > > > > + };
> > > > >
> > > > > Introducing private error codes always has the risk of just
> > > > > forwarding them as errno codes as done here.
> > > > >
> > > >
> > > > Yes, you're right.
> > > > We probably could do the same as scpi_to_linux_errno in
> > > > drivers/firmware/arm_scpi.c.
> > > > However, may can't fix the issue permanently.
> > > >
> > > > > > +
> > > > > > + of_node_put(mu_np);
> > > > > > +
> > > > > > + pr_info("NXP i.MX SCU Initialized (scu_ipc %u)\n",
> > > > > > +scu_ipc_handle);
> > > > > > +
> > > > > > + return 0;
> > > > > > +}
> > > > > > +early_initcall(imx8_scu_init);
> > > > >
> > > > > This code shows that the separate 'scu' device node shouldn't exist.
> > > > > It is only used as a stepping stone to find the 'mu' node.
> > > > > Instead of providing a proper driver for the 'mu' node the scu
> > > > > code registers it
> > > with its physical address.
> > > > > I don't think it makes sense to separate mu and scu code in this way.
> > > > >
> > > >
> > > > I agree that may not look quite well.
> > > > It mainly because we want to use the MU physical address as a MU ID.
> > > > (can't use virtual address as sc_ipc_id_t is u32 defined by SCU firmware.
> > > >
> > > > If you have any better suggestion please let me know.
> > >
> > > What I'm suggesting is a single node:
> > >
> > > scu at 5d1b0000 {
> > > compatible = "fsl,imx8qxp-scu";
> > > reg = <0x0 0x5d1b0000 0x0 0x10000>;
> > > };
> > >
> > > Attach your code to this one, without any further separation between
> > > mu and scu code.
> > >
> >
> > A bit confused. You're suggesting a single node here without mu or
> > mailbox node phandle in it? Then how SCU use MU?
>
> ioremap the address and mu_receive_msg()/mu_send_msg() on it, just like
> you do already.
>
> >
> > > We discussed this internally and came to the conclusion that the SCU
> > > is not a generic user of a MU. The MU is designed to work together
> > > with a piece of SRAM to form a mailbox system. Instead of working as
> > > designed the SCU morses the messages through the doorbell (what the
> > > MU really is). For anything that uses the MU in the way it is
> > > designed I would suggest using the mailbox interface from
> > > drivers/mailbox/ along with the binding from
> Documentation/devicetree/bindings/mailbox/mailbox.txt.
> > >
> > > In the way I suggest there is no need for any MU ID.
> > >
> >
> > So you're suggesting switch to use mailbox instead of private MU
> > library function calls?
> > Something like:
> > scu {
> > compatible = "nxp,imx8qxp-scu", "simple-bus";
> > mboxes = <&mailbox 0>;
> > }
> > Then IPC is implemented based on mailbox?
> >
> > As I replied Oleksij Rempel in another mail in this thread, we've
> > tried mailbox but got performance regression issue and need more time
> > to investigate whether it's really quite suitable for i.MX SCU
> > firmware as SCU handling message quite fast in micro seconds. (Mailbox
> > polling method has much more latency than current MU sample polling we
> > used) and we want to avoid the SCU firmware API changes.
>
> I am not suggesting to do mailboxing (using shared memory) for the SCU.
> I am also not suggesting any API update for the SCU communication.
> I am just mentioning that doing mailboxing is the way the MU was originally
> designed for. Looking at the reference manual I see many MUs on the i.MX8.
> I guess most of them are for communication between the different cores on
> the system. For these it's probably worth writing some generic MU driver.
> The way the MU is used for communication with the SCU though is so special
> that it's not worth writing a generic driver, so just integrate the MU access
> functions in the SCU code.
>
I understand it.
One problem of the way you suggested may be that:
If we doing like below, we may lose flexibility to change the MU used
for SCU firmware communication.
scu at 5d1b0000 {
compatible = "fsl,imx8qxp-scu";
reg = <0x0 0x5d1b0000 0x0 0x10000>;
};
And current design is that the system supports multiple MU channels used
by various users at the same time, e.g. SCU, Power Domain, Clock and others.
User can flexibly change it under their nodes: And each MU channel is protected
by their private lock and not affect each others.
e.g.
scu {
compatible = "nxp,imx8qxp-scu", "simple-bus";
fsl,mu = <&lsio_mu0>;
clk: clk {
compatible = "fsl,imx8qxp-clk";
#clock-cells = <1>;
};
iomuxc: iomuxc {
compatible = "fsl,imx8qxp-iomuxc";
fsl,mu = <&lsio_mu3>;
};
imx8qx-pm {
#address-cells = <1>;
#size-cells = <0>;
fsl,mu = <&lsio_mu4>;
.............
}
The default code only uses MU0 which is used by SCU.
The change may affect this design. Any ideas?
Do you think we can keep the current way?
Regards
Dong Aisheng
> Sascha
>
> --
> Pengutronix e.K. | |
> Industrial Linux Solutions |
> https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fww
> w.pengutronix.de%2F&data=02%7C01%7Caisheng.dong%40nxp.com%7C266
> 24a5c38e5488a80b708d5b0f3107b%7C686ea1d3bc2b4c6fa92cd99c5c301635%
> 7C0%7C0%7C636609480354404338&sdata=XP%2BYdt9I7zURrQun2jRbempLhC
> XrYtMVMb3dEWrZuro%3D&reserved=0 |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
> Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply
* [PATCH v10 05/18] KVM: arm64: Convert lazy FPSIMD context switch trap to C
From: Dave Martin @ 2018-05-24 8:54 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180524081220.GJ55598@C02W217FHV2R.local>
On Thu, May 24, 2018 at 10:12:20AM +0200, Christoffer Dall wrote:
> On Wed, May 23, 2018 at 08:35:13PM +0100, Alex Benn?e wrote:
> >
> > Dave Martin <Dave.Martin@arm.com> writes:
> >
> > > To make the lazy FPSIMD context switch trap code easier to hack on,
> > > this patch converts it to C.
> > >
> > > This is not amazingly efficient, but the trap should typically only
> > > be taken once per host context switch.
> > >
> > > Signed-off-by: Dave Martin <Dave.Martin@arm.com>
> > > Reviewed-by: Marc Zyngier <marc.zyngier@arm.com>
> > > ---
> > > arch/arm64/kvm/hyp/entry.S | 57 +++++++++++++++++----------------------------
> > > arch/arm64/kvm/hyp/switch.c | 24 +++++++++++++++++++
> > > 2 files changed, 46 insertions(+), 35 deletions(-)
[...]
> > > diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
> > > index d964523..c0796c4 100644
> > > --- a/arch/arm64/kvm/hyp/switch.c
> > > +++ b/arch/arm64/kvm/hyp/switch.c
> > > @@ -318,6 +318,30 @@ static bool __hyp_text __skip_instr(struct kvm_vcpu *vcpu)
> > > }
> > > }
> > >
> > > +void __hyp_text __hyp_switch_fpsimd(u64 esr __always_unused,
> > > + struct kvm_vcpu *vcpu)
> > > +{
> > > + kvm_cpu_context_t *host_ctxt;
> > > +
> > > + if (has_vhe())
> > > + write_sysreg(read_sysreg(cpacr_el1) | CPACR_EL1_FPEN,
> > > + cpacr_el1);
> > > + else
> > > + write_sysreg(read_sysreg(cptr_el2) & ~(u64)CPTR_EL2_TFP,
> > > + cptr_el2);
> >
> > Is there no way to do alternative() in C or does it always come down to
> > different inline asms?
> >
>
> has_vhe() should resolve to a static key, and I prefer this over the
> previous alternative construct we had for selecting function calls in C,
> as that resultet in having to follow too many levels of indirection.
I'll defer to Christoffer on that -- I was just following precedent :)
The if (has_vhe()) approach has the benefit of being much more
readable, and the static branch predictor in many CPUs will succeed in
folding a short-range unconditional branch out entirely. There will be
a small increase in I-cache pressure due to the larger inline code
size, but probably not much beyond that.
Cheers
---Dave
^ permalink raw reply
* [PATCH v3] arm64: allwinner: a64: Add Amarula A64-Relic initial support
From: Maxime Ripard @ 2018-05-24 8:54 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAMty3ZC-N-6WzYg+kYd2+V3wbQdQmvU706Nog_Urd_EUaWnQjQ@mail.gmail.com>
On Wed, May 23, 2018 at 03:57:05PM +0530, Jagan Teki wrote:
> On Wed, May 23, 2018 at 1:48 PM, Maxime Ripard
> <maxime.ripard@bootlin.com> wrote:
> > On Wed, May 23, 2018 at 11:44:56AM +0530, Jagan Teki wrote:
> >> On Tue, May 22, 2018 at 8:00 PM, Maxime Ripard
> >> <maxime.ripard@bootlin.com> wrote:
> >> > On Tue, May 22, 2018 at 06:52:28PM +0530, Jagan Teki wrote:
> >> >> Amarula A64-Relic is Allwinner A64 based IoT device, which support
> >> >> - Allwinner A64 Cortex-A53
> >> >> - Mali-400MP2 GPU
> >> >> - AXP803 PMIC
> >> >> - 1GB DDR3 RAM
> >> >> - 8GB eMMC
> >> >> - AP6330 Wifi/BLE
> >> >> - MIPI-DSI
> >> >> - CSI: OV5640 sensor
> >> >> - USB OTG
> >> >
> >> > You claim that this is doing OTG...
> >> >
> >> > [..]
> >> >
> >> >> +&usb_otg {
> >> >> + dr_mode = "peripheral";
> >> >> + status = "okay";
> >> >> +};
> >> >
> >> > ... and yet you're setting it as peripheral...
> >>
> >> Though it claims OTG, board doesn't have any USB ports to operate(not
> >> even Mini-AB) the only way to use the board as peripheral to transfer
> >> images from host.
> >
> > I'm not sure what you mean here. If there's no USB connector, why do
> > you even enable it?
>
> I'm saying there is no host port on board. Board has connector
> header[1] comes with few pins includes USB D+, D-, ID, VBUS, GROUND,
> UART0 TX, RX etc. we have to connect wires to these pins to make
> pluggable USB to host pc. and here USB pins used for transferring data
> from host pc to target board like in peripheral mode.
I'm still unsure how it's relevant. If the wire is plugged on your
board's header, is there anything that prevents using it as a host
assuming you have a proper connector on the other end of your wire?
Maxime
--
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180524/65f20e5f/attachment.sig>
^ permalink raw reply
* [PATCH 06/15] drm/sun4i: tcon: Add support for tcon-top
From: Maxime Ripard @ 2018-05-24 8:50 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <218132669.UY7RKz0VPx@jernej-laptop>
On Mon, May 21, 2018 at 07:27:46PM +0200, Jernej ?krabec wrote:
> Hi,
>
> Dne ponedeljek, 21. maj 2018 ob 10:07:59 CEST je Maxime Ripard napisal(a):
> > On Sat, May 19, 2018 at 08:31:18PM +0200, Jernej Skrabec wrote:
> > > If SoC has TCON TOP unit, it has to be configured from TCON, since it
> > > has all information needed. Additionally, if it is TCON TV, it must also
> > > enable bus gate inside TCON TOP unit.
> >
> > Why?
>
> I'll explain my design decision below.
>
> >
> > > Add support for such TCONs.
> > >
> > > Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
> > > ---
> > >
> > > drivers/gpu/drm/sun4i/sun4i_tcon.c | 28 ++++++++++++++++++++++++++++
> > > drivers/gpu/drm/sun4i/sun4i_tcon.h | 8 ++++++++
> > > 2 files changed, 36 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > b/drivers/gpu/drm/sun4i/sun4i_tcon.c index 08747fc3ee71..e0c562ce1c22
> > > 100644
> > > --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > @@ -688,6 +688,16 @@ static int sun4i_tcon_init_clocks(struct device *dev,
> > >
> > > dev_err(dev, "Couldn't get the TCON bus clock\n");
> > > return PTR_ERR(tcon->clk);
> > >
> > > }
> > >
> > > +
> > > + if (tcon->quirks->needs_tcon_top && tcon->quirks->has_channel_1) {
> > > + tcon->top_clk = devm_clk_get(dev, "tcon-top");
> > > + if (IS_ERR(tcon->top_clk)) {
> > > + dev_err(dev, "Couldn't get the TCON TOP bus clock\n");
> > > + return PTR_ERR(tcon->top_clk);
> > > + }
> > > + clk_prepare_enable(tcon->top_clk);
> > > + }
> > > +
> > >
> > > clk_prepare_enable(tcon->clk);
> > >
> > > if (tcon->quirks->has_channel_0) {
> > >
> > > @@ -712,6 +722,7 @@ static int sun4i_tcon_init_clocks(struct device *dev,
> > >
> > > static void sun4i_tcon_free_clocks(struct sun4i_tcon *tcon)
> > > {
> > >
> > > clk_disable_unprepare(tcon->clk);
> > >
> > > + clk_disable_unprepare(tcon->top_clk);
> > >
> > > }
> > >
> > > static int sun4i_tcon_init_irq(struct device *dev,
> > >
> > > @@ -980,6 +991,23 @@ static int sun4i_tcon_bind(struct device *dev, struct
> > > device *master,>
> > > tcon->id = engine->id;
> > > tcon->quirks = of_device_get_match_data(dev);
> > >
> > > + if (tcon->quirks->needs_tcon_top) {
> > > + struct device_node *np;
> > > +
> > > + np = of_parse_phandle(dev->of_node, "allwinner,tcon-top", 0);
> > > + if (np) {
> > > + struct platform_device *pdev;
> > > +
> > > + pdev = of_find_device_by_node(np);
> > > + if (pdev)
> > > + tcon->tcon_top = platform_get_drvdata(pdev);
> > > + of_node_put(np);
> > > +
> > > + if (!tcon->tcon_top)
> > > + return -EPROBE_DEFER;
> > > + }
> > > + }
> > > +
> >
> > I might have missed it, but I've not seen the bindings additions for
> > that property. This shouldn't really be done that way anyway, instead
> > of using a direct phandle, you should be using the of-graph, with the
> > TCON-top sitting where it belongs in the flow of data.
>
> Just to answer to the first question, it did describe it in "[PATCH 07/15] dt-
> bindings: display: sun4i-drm: Add R40 HDMI pipeline".
>
> As why I designed it that way - HW representation could be described that way
> (ASCII art makes sense when fixed width font is used to view it):
>
> / LCD0/LVDS0
> / TCON-LCD0
> | \ MIPI DSI
> mixer0 |
> \ / TCON-LCD1 - LCD1/LVDS1
> TCON-TOP
> / \ TCON-TV0 - TVE0/RGB
> mixer1 | \
> | TCON-TOP - HDMI
> | /
> \ TCON-TV1 - TVE1/RGB
>
> This is a bit simplified, since there is also TVE-TOP, which is responsible
> for sharing 4 DACs between both TVE encoders. You can have two TV outs (PAL/
> NTSC) or TVE0 as TV out and TVE1 as RGB or vice versa. It even seems that you
> can arbitrarly choose which DAC is responsible for which signal, so there is a
> ton of possible end combinations, but I'm not 100% sure.
>
> Even though I wrote TCON-TOP twice, this is same unit in HW. R40 manual
> suggest more possibilities, although some of them seem wrong, like RGB feeding
> from LCD TCON. That is confirmed to be wrong when checking BSP code.
>
> Additionally, TCON-TOP comes in the middle of TVE0 and LCD0, TVE1 and LCD1 for
> pin muxing, although I'm not sure why is that needed at all, since according
> to R40 datasheet, TVE0 and TVE1 pins are dedicated and not on PORT D and PORT
> H, respectively, as TCON-TOP documentation suggest. However, HSYNC and PSYNC
> lines might be shared between TVE (when it works in RGB mode) and LCD. But
> that is just my guess since I'm not really familiar with RGB and LCD
> interfaces.
>
> I'm really not sure what would be the best representation in OF-graph. Can you
> suggest one?
Rob might disagree on this one, but I don't see anything wrong with
having loops in the graph. If the TCON-TOP can be both the input and
output of the TCONs, then so be it, and have it described that way in
the graph.
The code is already able to filter out nodes that have already been
added to the list of devices we need to wait for in the component
framework, so that should work as well.
And we'd need to describe TVE-TOP as well, even though we don't have a
driver for it yet. That will simplify the backward compatibility later
on.
Maxime
--
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180524/768dfd71/attachment.sig>
^ permalink raw reply
* [PATCH v2 2/5] gpio: syscon: Add gpio-syscon for rockchip
From: Linus Walleij @ 2018-05-24 8:47 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <2044895.BqI5yRtle6@diego>
On Thu, May 24, 2018 at 10:35 AM, Heiko St?bner <heiko@sntech.de> wrote:
> Am Donnerstag, 24. Mai 2018, 10:28:44 CEST schrieb Linus Walleij:
>> On Wed, May 23, 2018 at 5:12 PM, Heiko St?bner <heiko@sntech.de> wrote:
>> > So the gpio controller should definitly also be a subnode.
>> >
>> > The gpio in question is called "mute", so I'd think the gpio-syscon driver
>> > should just define a "rockchip,rk3328-gpio-mute" compatible and contain
>> > all the register voodoo in the driver itself and not define it in the dt.
>> >
>> > So it should probably look like
>> >
>> > grf: syscon at ff100000 {
>> >
>> > compatible = "rockchip,rk3328-grf", "syscon", "simple-mfd";
>> >
>> > [all the other syscon sub-devices]
>> >
>> > gpio_mute: gpio-mute {
>> >
>> > compatible = "rockchip,rk3328-gpio-mute";
>> > gpio-controller;
>> > #gpio-cells = <2>;
>> >
>> > };
>>
>> I'm sceptic.
>>
>> That doesn't sound like "general purpose input output" at all.
>>
>> It sounds like special purpose, for a mute button.
>>
>> Does it use IRQ? I would recommend implementing
>> drivers/input/keyboard/syscon-keys.c in the same vein
>> as drivers/leds/leds-syscon.c so you can avoid indirection
>> through GPIO for no good reason at all.
>
> To quote Levin from the other mail:
> --------
> The "mute" pin is a output only GPIO, which is already supported by
> setting flags in the gpio-syscon
> driver. And yes, this pin has a defined function, but can also be used
> for general purpose operation.
> --------
>
> So to summarize, the documentation calls it "mute", but it is usable as
> a general pin, which is the reason Levin is working on it - because on his
> board this pin is used to switch between two voltages (aka a gpio-regulator)
> for the sdmmc controller [3.3V + 1.8V].
OK then, I was wrong! :)
Go ahead with this, sorry for the fuzz.
Yours,
Linus Walleij
^ permalink raw reply
* [PATCH 05/15] drm/sun4i: Add TCON TOP driver
From: Maxime Ripard @ 2018-05-24 8:43 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <5965231.6ucpJrPIQ5@jernej-laptop>
Hi,
On Mon, May 21, 2018 at 05:15:15PM +0200, Jernej ?krabec wrote:
> > > + /*
> > > + * Default register values might have some reserved bits set, which
> > > + * prevents TCON TOP from working properly. Set them to 0 here.
> > > + */
> > > + writel(0, tcon_top->regs + TCON_TOP_PORT_SEL_REG);
> > > + writel(0, tcon_top->regs + TCON_TOP_GATE_SRC_REG);
> > > +
> > > + for (i = 0; i < CLK_NUM; i++) {
> > > + const char *parent_name = "bus-tcon-top";
> >
> > I guess retrieving the parent's clock name at runtime would be more
> > flexible.
>
> It is, but will it ever be anything else?
Probably not, but when the complexity is exactly the same (using
__clk_get_name), we'd better use the more appropriate solution. If we
ever need to change that clock name, or to use the driver with an SoC
that wouldn't have the same clock name for whatever reason, it will
just work.
> > > + struct clk_init_data init;
> > > + struct clk_gate *gate;
> > > +
> > > + gate = devm_kzalloc(dev, sizeof(*gate), GFP_KERNEL);
> > > + if (!gate) {
> > > + ret = -ENOMEM;
> > > + goto err_disable_clock;
> > > + }
> > > +
> > > + init.name = gates[i].name;
> > > + init.ops = &clk_gate_ops;
> > > + init.flags = CLK_IS_BASIC;
> > > + init.parent_names = &parent_name;
> > > + init.num_parents = 1;
> > > +
> > > + gate->reg = tcon_top->regs + TCON_TOP_GATE_SRC_REG;
> > > + gate->bit_idx = gates[i].bit;
> > > + gate->lock = &tcon_top->reg_lock;
> > > + gate->hw.init = &init;
> > > +
> > > + ret = devm_clk_hw_register(dev, &gate->hw);
> > > + if (ret)
> > > + goto err_disable_clock;
> >
> > Isn't it what clk_hw_register_gate is doing?
>
> Almost, but not exactly. My goal was to use devm_* functions, so there is no
> need to do any special cleanup.
Is it the only difference? If so, you can just create a
devm_clk_hw_register gate.
Maxime
--
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180524/6ef507c2/attachment-0001.sig>
^ permalink raw reply
* [PATCH v2 2/4] drm/mediatek: Add support for mediatek SOC MT2712
From: Stu Hsieh @ 2018-05-24 8:40 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1527125197.6406.10.camel@mtksdaap41>
Hi CK,
On Thu, 2018-05-24 at 09:26 +0800, CK Hu wrote:
> Hi, Stu:
>
> On Wed, 2018-05-23 at 17:28 +0800, Stu Hsieh wrote:
> > On Wed, 2018-05-23 at 13:23 +0800, CK Hu wrote:
> > > Hi, Stu:
> > >
> > > I've some inline comment.
> > >
> > > On Wed, 2018-05-23 at 10:25 +0800, Stu Hsieh wrote:
> > > > This patch add support for the Mediatek MT2712 DISP subsystem.
> > > > There are two OVL engine and three disp output in MT2712.
> > > >
> > > > Signed-off-by: Stu Hsieh <stu.hsieh@mediatek.com>
> > > > ---
> > > > drivers/gpu/drm/mediatek/mtk_drm_ddp.c | 50 +++++++++++++++++++++++++++--
> > > > drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c | 8 +++--
> > > > drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h | 7 ++--
> > > > drivers/gpu/drm/mediatek/mtk_drm_drv.c | 47 +++++++++++++++++++++++++--
> > > > drivers/gpu/drm/mediatek/mtk_drm_drv.h | 7 ++--
> > > > 5 files changed, 108 insertions(+), 11 deletions(-)
> > > >
> > > > +#define MT2712_MUTEX_MOD_DISP_AAL0 BIT(20)
> > > > +#define MT2712_MUTEX_MOD_DISP_UFOE BIT(22)
> > > > +#define MT2712_MUTEX_MOD_DISP_PWM0 BIT(23)
> > > > +#define MT2712_MUTEX_MOD_DISP_PWM1 BIT(24)
> > > > +#define MT2712_MUTEX_MOD_DISP_PWM2 BIT(10)
> > > > +#define MT2712_MUTEX_MOD_DISP_OD0 BIT(25)
> > > > +/* modules more than 32, add BIT(31) when using DISP_REG_MUTEX_MOD2 bit */
> > > > +#define MT2712_MUTEX_MOD2_DISP_AAL1 (BIT(1) | BIT(31))
> > >
> > > I think a better definition is
> > >
> > > #define MT2712_MUTEX_MOD2_DISP_AAL1 BIT(33)
> > >
> > > when you need to access this register,
> > >
> > > if (ddp->mutex_mod[id] < BIT(32)) {
> > > offset = DISP_REG_MUTEX_MOD(mutex->id);
> > > reg = readl_relaxed(ddp->regs + offset);
> > > reg |= ddp->mutex_mod[id];
> > > writel_relaxed(reg, ddp->regs + offset);
> > > } else {
> > > offset = DISP_REG_MUTEX_MOD2(mutex->id);
> > > reg = readl_relaxed(ddp->regs + offset);
> > > reg |= (ddp->mutex_mod[id] >> 32);
> > > writel_relaxed(reg, ddp->regs + offset);
> > > }
> > >
> > > because DISP_REG_MUTEX_MOD BIT(31) could be used for some module.
> >
> > This modification is workable, but result some build warning like
> > following:
> > 1. #define BIT(nr) (1UL << (nr)) in include/linux/bitops.h
> > 2. [DDP_COMPONENT_AAL1] = MT2712_MUTEX_MOD2_DISP_AAL1,
> > => we need to modify the definition about "static const unsigned int
> > mt2712_mutex_mod"
> >
>
> Currently, mutex_mod is a bitwise definition. I think it could be
> changed to index definition such as
>
>
> #define MT2712_MUTEX_MOD_DISP_PWM2 10
> #define MT2712_MUTEX_MOD_DISP_OD0 25
> #define MT2712_MUTEX_MOD2_DISP_AAL1 33
>
> when you need to access this register,
>
> if (ddp->mutex_mod[id] < 32) {
> offset = DISP_REG_MUTEX_MOD(mutex->id);
> reg = readl_relaxed(ddp->regs + offset);
> reg |= 1 << ddp->mutex_mod[id];
> writel_relaxed(reg, ddp->regs + offset);
> } else {
> offset = DISP_REG_MUTEX_MOD2(mutex->id);
> reg = readl_relaxed(ddp->regs + offset);
> reg |= 1 << (ddp->mutex_mod[id] - 32);
> writel_relaxed(reg, ddp->regs + offset);
> }
>
> Regards,
> CK
This modification has no build warning.
I would also change the definition about 2701 and 8173 from bitwise to
index.
Regards,
Stu
>
> > > > +#define MT2712_MUTEX_MOD2_DISP_OD1 (BIT(2) | BIT(31))
> > > > +
> > > > #define MT2701_MUTEX_MOD_DISP_OVL BIT(3)
> > > > #define MT2701_MUTEX_MOD_DISP_WDMA BIT(6)
> > > > #define MT2701_MUTEX_MOD_DISP_COLOR BIT(7)
> > > > @@ -74,6 +96,7 @@
> > > >
> > > >
> > >
> > >
> >
> >
>
>
^ permalink raw reply
* [PATCHv2] arm64: Make sure permission updates happen for pmd/pud
From: Peter Robinson @ 2018-05-24 8:38 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180523184346.487-1-labbott@redhat.com>
On Wed, May 23, 2018 at 7:43 PM, Laura Abbott <labbott@redhat.com> wrote:
> Commit 15122ee2c515 ("arm64: Enforce BBM for huge IO/VMAP mappings")
> disallowed block mappings for ioremap since that code does not honor
> break-before-make. The same APIs are also used for permission updating
> though and the extra checks prevent the permission updates from happening,
> even though this should be permitted. This results in read-only permissions
> not being fully applied. Visibly, this can occasionaly be seen as a failure
> on the built in rodata test when the test data ends up in a section or
> as an odd RW gap on the page table dump. Fix this by using
> pgattr_change_is_safe instead of p*d_present for determining if the
> change is permitted.
>
> Reported-by: Peter Robinson <pbrobinson@gmail.com>
> Fixes: 15122ee2c515 ("arm64: Enforce BBM for huge IO/VMAP mappings")
> Signed-off-by: Laura Abbott <labbott@redhat.com>
Tested-by: Peter Robinson <pbrobinson@gmail.com>
Tested on Macbin, mustang, pine64, RPi3+ and db410c and fixes the issue I saw.
> ---
> v2: Switch to using pgattr_change_is_safe per suggestion of Will
> ---
> arch/arm64/mm/mmu.c | 16 ++++++++++------
> 1 file changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> index 2dbb2c9f1ec1..493ff75670ff 100644
> --- a/arch/arm64/mm/mmu.c
> +++ b/arch/arm64/mm/mmu.c
> @@ -933,13 +933,15 @@ int pud_set_huge(pud_t *pudp, phys_addr_t phys, pgprot_t prot)
> {
> pgprot_t sect_prot = __pgprot(PUD_TYPE_SECT |
> pgprot_val(mk_sect_prot(prot)));
> + pud_t new_pud = pfn_pud(__phys_to_pfn(phys), sect_prot);
>
> - /* ioremap_page_range doesn't honour BBM */
> - if (pud_present(READ_ONCE(*pudp)))
> + /* Only allow permission changes for now */
> + if (!pgattr_change_is_safe(READ_ONCE(pud_val(*pudp)),
> + pud_val(new_pud)))
> return 0;
>
> BUG_ON(phys & ~PUD_MASK);
> - set_pud(pudp, pfn_pud(__phys_to_pfn(phys), sect_prot));
> + set_pud(pudp, new_pud);
> return 1;
> }
>
> @@ -947,13 +949,15 @@ int pmd_set_huge(pmd_t *pmdp, phys_addr_t phys, pgprot_t prot)
> {
> pgprot_t sect_prot = __pgprot(PMD_TYPE_SECT |
> pgprot_val(mk_sect_prot(prot)));
> + pmd_t new_pmd = pfn_pmd(__phys_to_pfn(phys), sect_prot);
>
> - /* ioremap_page_range doesn't honour BBM */
> - if (pmd_present(READ_ONCE(*pmdp)))
> + /* Only allow permission changes for now */
> + if (!pgattr_change_is_safe(READ_ONCE(pmd_val(*pmdp)),
> + pmd_val(new_pmd)))
> return 0;
>
> BUG_ON(phys & ~PMD_MASK);
> - set_pmd(pmdp, pfn_pmd(__phys_to_pfn(phys), sect_prot));
> + set_pmd(pmdp, new_pmd);
> return 1;
> }
>
> --
> 2.17.0
>
^ permalink raw reply
* [PATCH 2/2] soc: imx: add SCU power domains driver
From: A.s. Dong @ 2018-05-24 8:37 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAPDyKFowdpxwNMmDjdD3qcqnAcQZA9-MYWnTcZL-oSbwg8WYMg@mail.gmail.com>
Hi Ulf,
Thanks for the review.
> -----Original Message-----
> From: Ulf Hansson [mailto:ulf.hansson at linaro.org]
> Sent: Wednesday, May 9, 2018 3:16 AM
> To: A.s. Dong <aisheng.dong@nxp.com>
> Cc: Linux ARM <linux-arm-kernel@lists.infradead.org>; Dong Aisheng
> <dongas86@gmail.com>; Sascha Hauer <kernel@pengutronix.de>; Shawn
> Guo <shawnguo@kernel.org>; Fabio Estevam <fabio.estevam@nxp.com>;
> dl-linux-imx <linux-imx@nxp.com>; Rafael J. Wysocki <rjw@rjwysocki.net>;
> Kevin Hilman <khilman@kernel.org>; Linux PM <linux-pm@vger.kernel.org>
> Subject: Re: [PATCH 2/2] soc: imx: add SCU power domains driver
>
> [...]
>
> > +
> > +static int __init imx_sc_init_pm_domains(void) {
> > + struct generic_pm_domain *pd;
> > + struct device_node *np;
> > + sc_err_t sci_err;
> > +
> > + if (!of_machine_is_compatible("fsl,imx8qxp"))
> > + return 0;
> > +
> > + sci_err = sc_ipc_get_handle(&pm_ipc_handle);
> > + if (sci_err != SC_ERR_NONE) {
> > + pr_err("imx_sc_pd: can't get sc ipc handle\n");
> > + return -ENODEV;
> > + }
> > +
> > + for_each_matching_node(np, imx_sc_pm_domain_of_match) {
> > + pd = imx_sc_pm_add_one_domain(np, NULL);
> > + if (!IS_ERR(pd))
> > + imx_sc_pm_add_subdomains(np, pd);
> > + }
>
> Perhaps using of_genpd_add_subdomain() may help here and possibly could
> avoid some open coding!?
>
Thanks for the suggestion. I thought of it a lot and the result is that I'm not sure
If it's quite suitable for i.MX cases. Currently seems there's only one user, Samsung,
in kernel using that API which takes two struct of_phandle_args as arguments,
parent and child. It looks needs special handling in code before using it, e.g.
register both parent and child domain first, which is somehow not like i.MX flow
of registration. It looks like to me not see much benefits to enforce a big
change to switch to it. Or am I missed anything?
If you have better idea please let me know.
> > +
> > + return 0;
> > +}
> > +early_initcall(imx_sc_init_pm_domains);
>
> Otherwise this looks good to me!
>
Thanks
Regards
Dong Aisheng
> Kind regards
> Uffe
^ permalink raw reply
* [PATCH v2 2/5] gpio: syscon: Add gpio-syscon for rockchip
From: Heiko Stübner @ 2018-05-24 8:35 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CACRpkdYCdNZ=ASw1uz4zKXC4AMd1EGbvG_G5K5cwSpH5eGPgKA@mail.gmail.com>
Hi Linus,
Am Donnerstag, 24. Mai 2018, 10:28:44 CEST schrieb Linus Walleij:
> On Wed, May 23, 2018 at 5:12 PM, Heiko St?bner <heiko@sntech.de> wrote:
> > So the gpio controller should definitly also be a subnode.
> >
> > The gpio in question is called "mute", so I'd think the gpio-syscon driver
> > should just define a "rockchip,rk3328-gpio-mute" compatible and contain
> > all the register voodoo in the driver itself and not define it in the dt.
> >
> > So it should probably look like
> >
> > grf: syscon at ff100000 {
> >
> > compatible = "rockchip,rk3328-grf", "syscon", "simple-mfd";
> >
> > [all the other syscon sub-devices]
> >
> > gpio_mute: gpio-mute {
> >
> > compatible = "rockchip,rk3328-gpio-mute";
> > gpio-controller;
> > #gpio-cells = <2>;
> >
> > };
>
> I'm sceptic.
>
> That doesn't sound like "general purpose input output" at all.
>
> It sounds like special purpose, for a mute button.
>
> Does it use IRQ? I would recommend implementing
> drivers/input/keyboard/syscon-keys.c in the same vein
> as drivers/leds/leds-syscon.c so you can avoid indirection
> through GPIO for no good reason at all.
To quote Levin from the other mail:
--------
The "mute" pin is a output only GPIO, which is already supported by
setting flags in the gpio-syscon
driver. And yes, this pin has a defined function, but can also be used
for general purpose operation.
--------
So to summarize, the documentation calls it "mute", but it is usable as
a general pin, which is the reason Levin is working on it - because on his
board this pin is used to switch between two voltages (aka a gpio-regulator)
for the sdmmc controller [3.3V + 1.8V].
Available pin settings are output-enable + of course the high/low setting
and I think I remember there is even a pull setting for it in the GRF
somewhere - but my memory might be fuzzy here.
Heiko
^ permalink raw reply
* [PATCH v10 07/18] arm64: fpsimd: Eliminate task->mm checks
From: Christoffer Dall @ 2018-05-24 8:33 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180523150337.GO13470@e103592.cambridge.arm.com>
On Wed, May 23, 2018 at 04:03:37PM +0100, Dave Martin wrote:
> On Wed, May 23, 2018 at 03:56:57PM +0100, Catalin Marinas wrote:
> > On Wed, May 23, 2018 at 02:31:59PM +0100, Dave P Martin wrote:
> > > On Wed, May 23, 2018 at 01:48:12PM +0200, Christoffer Dall wrote:
> > > > On Tue, May 22, 2018 at 05:05:08PM +0100, Dave Martin wrote:
> > > > > This is true by construction however: TIF_FOREIGN_FPSTATE is never
> > > > > cleared except when returning to userspace or returning from a
> > > > > signal: thus, for a true kernel thread no FPSIMD context is ever
> > > > > loaded, TIF_FOREIGN_FPSTATE will remain set and no context will
> > > > > ever be saved.
> > > >
> > > > I don't understand this construction proof; from looking at the patch
> > > > below it is not obvious to me why fpsimd_thread_switch() can never have
> > > > !wrong_task && !wrong_cpu and therefore clear TIF_FOREIGN_FPSTATE for a
> > > > kernel thread?
> > >
> > > Looking at this again, I think it is poorly worded. This patch aims to
> > > make it true by construction, but it isn't prior to the patch.
> > >
> > > I'm tempted to delete the paragraph: the assertion of both untrue and
> > > not the best way to justify that this patch works.
> > >
> > >
> > > How about:
> > >
> > > -8<-
> > >
> > > The context switch logic already isolates user threads from each other.
> > > This, it is sufficient for isolating user threads from the kernel,
s/This/Thus/ ?
I don't understand what 'it' refers to here?
> > > since the goal either way is to ensure that code executing in userspace
> > > cannot see any FPSIMD state except its own. Thus, there is no special
> > > property of kernel threads that we care about except that it is
> > > pointless to save or load FPSIMD register state for them.
Actually, I'm not really sure what this paragraph is getting at.
> > >
> > > At worst, the removal of all the kernel thread special cases by this
> > > patch would thus spuriously load and save state for kernel threads when
> > > unnecessary.
> > >
> > > But the context switch logic is already deliberately optimised to defer
> > > reloads of the regs until ret_to_user (or sigreturn as a special case),
> > > which kernel threads by definition never reach.
> > >
> > > ->8-
> >
> > The "at worst" paragraph makes it look like it could happen (at least
> > until you reach the last paragraph). Maybe you can just say that
> > wrong_task and wrong_cpu (with the fpsimd_cpu = NR_CPUS addition) are
> > always true for kernel threads. You should probably mention this in a
> > comment in the code as well.
>
> What if I just delete the second paragraph, and remove the "But" from
> the start of the third, and append:
>
> "As a result, the wrong_task and wrong_cpu tests in
> fpsimd_thread_switch() will always yield false for kernel threads."
>
> ...with a similar comment in the code?
...with a risk of being a bit over-pedantic and annoying, may I suggest
the following complete commit text:
------8<------
Currently the FPSIMD handling code uses the condition task->mm ==
NULL as a hint that task has no FPSIMD register context.
The ->mm check is only there to filter out tasks that cannot
possibly have FPSIMD context loaded, for optimisation purposes.
However, TIF_FOREIGN_FPSTATE must always be checked anyway before
saving FPSIMD context back to memory. For this reason, the ->mm
checks are not useful, providing that that TIF_FOREIGN_FPSTATE is
maintained properly for kernel threads.
FPSIMD context is never preserved for kernel threads across a context
switch and therefore TIF_FOREIGN_FPSTATE should always be true for
kernel threads. This is indeed the case, as the wrong_task and
wrong_cpu tests in fpsimd_thread_switch() will always yield false for
kernel threads.
Further, the context switch logic is already deliberately optimised to
defer reloads of the FPSIMD context until ret_to_user (or sigreturn as a
special case), which kernel threads by definition never reach, and
therefore this change introduces no additional work in the critical
path.
This patch removes the redundant checks and special-case code.
------8<------
Thanks,
-Christoffer
^ permalink raw reply
* [PATCH] PM / runtime: Drop usage count for suppliers at device link removal
From: Ulf Hansson @ 2018-05-24 8:33 UTC (permalink / raw)
To: linux-arm-kernel
In the case consumer device is runtime resumed, while the link to the
supplier is removed, the earlier call to pm_runtime_get_sync() made from
rpm_get_suppliers() does not get properly balanced with a corresponding
call to pm_runtime_put(). This leads to that suppliers remains to be
runtime resumed forever, while they don't need to.
Let's fix the behaviour by calling rpm_put_suppliers() when dropping a
device link. Not that, since rpm_put_suppliers() checks the
link->rpm_active flag, we can correctly avoid to call pm_runtime_put() in
cases when we shouldn't.
Reported-by: Todor Tomov <todor.tomov@linaro.org>
Fixes: 21d5c57b3726 ("PM / runtime: Use device links")
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
Rafael, I am not sure if this is safe from locking point of view. The device
link write lock has been taken when pm_runtime_drop_link() is called, hence I
assume calling rpm_put_suppliers() should be fine!? If not, can you please
advise how to change?
Kind regards
Uffe
---
drivers/base/power/runtime.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c
index 8bef3cb..beb85c3 100644
--- a/drivers/base/power/runtime.c
+++ b/drivers/base/power/runtime.c
@@ -1607,6 +1607,8 @@ void pm_runtime_new_link(struct device *dev)
void pm_runtime_drop_link(struct device *dev)
{
+ rpm_put_suppliers(dev);
+
spin_lock_irq(&dev->power.lock);
WARN_ON(dev->power.links_count == 0);
dev->power.links_count--;
--
2.7.4
^ permalink raw reply related
* [PATCH 1/5] pinctrl: at91-pio4: add missing of_node_put
From: Linus Walleij @ 2018-05-24 8:30 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1527102436-13447-2-git-send-email-Julia.Lawall@lip6.fr>
On Wed, May 23, 2018 at 9:07 PM, Julia Lawall <Julia.Lawall@lip6.fr> wrote:
> The device node iterators perform an of_node_get on each iteration, so a
> jump out of the loop requires an of_node_put.
>
> The semantic patch that fixes this problem is as follows
> (http://coccinelle.lip6.fr):
>
> // <smpl>
> @@
> expression root,e;
> local idexpression child;
> iterator name for_each_child_of_node;
> @@
>
> for_each_child_of_node(root, child) {
> ... when != of_node_put(child)
> when != e = child
> + of_node_put(child);
> ? break;
> ...
> }
> ... when != child
> // </smpl>
>
> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
Patch applied with Ludovic's ACK!
Yours,
Linus Walleij
^ permalink raw reply
* [PATCH v2 2/5] gpio: syscon: Add gpio-syscon for rockchip
From: Linus Walleij @ 2018-05-24 8:28 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1685755.J6GI985WX3@diego>
On Wed, May 23, 2018 at 5:12 PM, Heiko St?bner <heiko@sntech.de> wrote:
> So the gpio controller should definitly also be a subnode.
>
> The gpio in question is called "mute", so I'd think the gpio-syscon driver
> should just define a "rockchip,rk3328-gpio-mute" compatible and contain
> all the register voodoo in the driver itself and not define it in the dt.
>
> So it should probably look like
>
> grf: syscon at ff100000 {
> compatible = "rockchip,rk3328-grf", "syscon", "simple-mfd";
>
> [all the other syscon sub-devices]
>
> gpio_mute: gpio-mute {
> compatible = "rockchip,rk3328-gpio-mute";
> gpio-controller;
> #gpio-cells = <2>;
> };
I'm sceptic.
That doesn't sound like "general purpose input output" at all.
It sounds like special purpose, for a mute button.
Does it use IRQ? I would recommend implementing
drivers/input/keyboard/syscon-keys.c in the same vein
as drivers/leds/leds-syscon.c so you can avoid indirection
through GPIO for no good reason at all.
I already have other good uses for such a generic
input driver.
Yours,
Linus Walleij
^ permalink raw reply
* [linux-sunxi] Re: [PATCH 12/15] drm/sun4i: Add support for second clock parent to DW HDMI PHY clk driver
From: Maxime Ripard @ 2018-05-24 8:27 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <4018914.loHx9iTxYh@jernej-laptop>
On Mon, May 21, 2018 at 05:02:23PM +0200, Jernej ?krabec wrote:
> Hi,
>
> Dne ponedeljek, 21. maj 2018 ob 10:12:53 CEST je Maxime Ripard napisal(a):
> > On Sat, May 19, 2018 at 08:31:24PM +0200, Jernej Skrabec wrote:
> > > Expand HDMI PHY clock driver to support second clock parent.
> > >
> > > Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
> > > ---
> > >
> > > drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h | 6 +-
> > > drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c | 29 ++++++-
> > > drivers/gpu/drm/sun4i/sun8i_hdmi_phy_clk.c | 90 ++++++++++++++++------
> > > 3 files changed, 98 insertions(+), 27 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h
> > > b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h index 801a17222762..aadbe0a10b0c
> > > 100644
> > > --- a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h
> > > +++ b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h
> > > @@ -98,7 +98,8 @@
> > >
> > > #define SUN8I_HDMI_PHY_PLL_CFG1_LDO2_EN BIT(29)
> > > #define SUN8I_HDMI_PHY_PLL_CFG1_LDO1_EN BIT(28)
> > > #define SUN8I_HDMI_PHY_PLL_CFG1_HV_IS_33 BIT(27)
> > >
> > > -#define SUN8I_HDMI_PHY_PLL_CFG1_CKIN_SEL BIT(26)
> > > +#define SUN8I_HDMI_PHY_PLL_CFG1_CKIN_SEL_MSK BIT(26)
> > > +#define SUN8I_HDMI_PHY_PLL_CFG1_CKIN_SEL_SHIFT 26
> > >
> > > #define SUN8I_HDMI_PHY_PLL_CFG1_PLLEN BIT(25)
> > > #define SUN8I_HDMI_PHY_PLL_CFG1_LDO_VSET(x) ((x) << 22)
> > > #define SUN8I_HDMI_PHY_PLL_CFG1_UNKNOWN(x) ((x) << 20)
> > >
> > > @@ -190,6 +191,7 @@ void sun8i_hdmi_phy_remove(struct sun8i_dw_hdmi
> > > *hdmi);
> > >
> > > void sun8i_hdmi_phy_init(struct sun8i_hdmi_phy *phy);
> > > const struct dw_hdmi_phy_ops *sun8i_hdmi_phy_get_ops(void);
> > >
> > > -int sun8i_phy_clk_create(struct sun8i_hdmi_phy *phy, struct device *dev);
> > > +int sun8i_phy_clk_create(struct sun8i_hdmi_phy *phy, struct device *dev,
> > > + bool second_parent);
> > >
> > > #endif /* _SUN8I_DW_HDMI_H_ */
> > >
> > > diff --git a/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c
> > > b/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c index deba47ed69d8..7a911f0a3ae3
> > > 100644
> > > --- a/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c
> > > +++ b/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c
> > > @@ -183,7 +183,13 @@ static int sun8i_hdmi_phy_config_h3(struct dw_hdmi
> > > *hdmi,>
> > > regmap_update_bits(phy->regs, SUN8I_HDMI_PHY_ANA_CFG1_REG,
> > >
> > > SUN8I_HDMI_PHY_ANA_CFG1_TXEN_MASK, 0);
> > >
> > > - regmap_write(phy->regs, SUN8I_HDMI_PHY_PLL_CFG1_REG, pll_cfg1_init);
> > > + /*
> > > + * NOTE: We have to be careful not to overwrite PHY parent
> > > + * clock selection bit and clock divider.
> > > + */
> > > + regmap_update_bits(phy->regs, SUN8I_HDMI_PHY_PLL_CFG1_REG,
> > > + ~SUN8I_HDMI_PHY_PLL_CFG1_CKIN_SEL_MSK,
> > > + pll_cfg1_init);
> >
> > It feels like it belongs in a separate patch.
>
> Why? clk_set_rate() on HDMI PHY clock is called before this function, so
> SUN8I_HDMI_PHY_PLL_CFG1_CKIN_SEL bit is already set to correct value. Original
> code doesn't take this into account and sets it to 0 here in all cases, which
> obviously is not right.
>
> If you insist on splitting this in new patch, it has to be applied before
> clock changes. It would also need to include "reset PLL clock configuration"
> chunk (next change in this patch), otherwise other SoCs with only one parent
> could get 1 there, which is obviously wrong. Note that I didn't really tested
> if default value of this bit is 0 or 1, but I wouldn't really like to depend
> on default values.
I don't have a strong feeling about this, but to me, the fact that you
don't want to overwrite the muxing bit because the clock might use it
is sensible in itself, disregarding the fact that the clock *will* use
it.
>
> >
> > > regmap_update_bits(phy->regs, SUN8I_HDMI_PHY_PLL_CFG2_REG,
> > >
> > > (u32)~SUN8I_HDMI_PHY_PLL_CFG2_PREDIV_MSK,
> > > pll_cfg2_init);
> > >
> > > @@ -352,6 +358,10 @@ static void sun8i_hdmi_phy_init_h3(struct
> > > sun8i_hdmi_phy *phy)>
> > > SUN8I_HDMI_PHY_ANA_CFG3_SCLEN |
> > > SUN8I_HDMI_PHY_ANA_CFG3_SDAEN);
> > >
> > > + /* reset PLL clock configuration */
> > > + regmap_write(phy->regs, SUN8I_HDMI_PHY_PLL_CFG1_REG, 0);
> > > + regmap_write(phy->regs, SUN8I_HDMI_PHY_PLL_CFG2_REG, 0);
> > > +
> >
> > Ditto,
> >
> > > +
> > > + /*
> > > + * Even though HDMI PHY clock doesn't have enable/disable
> > > + * handlers, we have to enable it. Otherwise it could happen
> > > + * that parent PLL is not enabled by clock framework in a
> > > + * highly unlikely event when parent PLL is used solely for
> > > + * HDMI PHY clock.
> > > + */
> > > + clk_prepare_enable(phy->clk_phy);
> >
> > The implementation of the clock doesn't really matter in our API
> > usage. If we're using a clock, we have to call
> > clk_prepare_enable. That's documented everywhere, and mentionning how
> > the clock is implemented is an abstraction leakage and it's
> > irrelevant. I'd simply remove the comment here.
> >
> > And it should be in a separate patch as well.
>
> OK. Should I add Fixes tag, since current code obviously is not by the specs?
> It could still get in 4.17...
Yep!
Maxime
--
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180524/374eaf4a/attachment.sig>
^ permalink raw reply
* [PATCH v4 3/3] arm64: Introduce command line parameter to disable CNP
From: Vladimir Murzin @ 2018-05-24 8:20 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180523171721.ppazcyf576koimhl@armageddon.cambridge.arm.com>
On 23/05/18 18:17, Catalin Marinas wrote:
> On Fri, May 18, 2018 at 11:07:02AM +0100, Vladimir Murzin wrote:
>> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
>> index 11fc28e..8f59d47 100644
>> --- a/Documentation/admin-guide/kernel-parameters.txt
>> +++ b/Documentation/admin-guide/kernel-parameters.txt
>> @@ -2636,6 +2636,10 @@
>>
>> noclflush [BUGS=X86] Don't use the CLFLUSH instruction
>>
>> + nocnp [ARM64]
>> + Disable CNP (Common not Private translations)
>> + even if it is supported by processor.
>> +
>> nodelayacct [KNL] Disable per-task delay accounting
>>
>> nodsp [SH] Disable hardware DSP at boot time.
>
> Do we actually have a use-case for this command line option? I'm not
> considering hardware errata as these are handled separately in the
> kernel.
>
Well, I cannot count all cases, yet we might see CnP support advertised
by CPU via ID register (where CPU meant to be part of bL) but not really
doing optimisations in hardware.
Probably, some userspace (benchmarks) might not benefit of CnP; otoh maybe
better way for such case would be user-space asking kernel to {dis,en}able
CnP...
I have no strong opinion on patch, so I'm fine to drop it and come back
when/if we get results from real hardware.
Cheers
Vladimir
^ permalink raw reply
* [PATCH] pinctrl: armada-37xx: Fix spurious irq management
From: Linus Walleij @ 2018-05-24 8:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180523084405.14410-1-gregory.clement@bootlin.com>
On Wed, May 23, 2018 at 10:44 AM, Gregory CLEMENT
<gregory.clement@bootlin.com> wrote:
> From: Terry Zhou <bjzhou@marvell.com>
>
> Until now, if we found spurious irq in irq_handler, we only updated the
> status in register but not the status in the code. Due to this the system
> will got stuck dues to the infinite loop
>
> [gregory.clement at bootlin.com: update comment and add fix and stable tags]
> Fixes: 30ac0d3b0702 ("pinctrl: armada-37xx: Add edge both type gpio irq support")
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Terry Zhou <bjzhou@marvell.com>
> Reviewed-by: Gregory CLEMENT <gregory.clement@bootlin.com>
> Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com>
Patch applied.
Yours,
Linus Walleij
^ permalink raw reply
* [PATCH v10 05/18] KVM: arm64: Convert lazy FPSIMD context switch trap to C
From: Christoffer Dall @ 2018-05-24 8:12 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <87k1rutbbi.fsf@linaro.org>
On Wed, May 23, 2018 at 08:35:13PM +0100, Alex Benn?e wrote:
>
> Dave Martin <Dave.Martin@arm.com> writes:
>
> > To make the lazy FPSIMD context switch trap code easier to hack on,
> > this patch converts it to C.
> >
> > This is not amazingly efficient, but the trap should typically only
> > be taken once per host context switch.
> >
> > Signed-off-by: Dave Martin <Dave.Martin@arm.com>
> > Reviewed-by: Marc Zyngier <marc.zyngier@arm.com>
> > ---
> > arch/arm64/kvm/hyp/entry.S | 57 +++++++++++++++++----------------------------
> > arch/arm64/kvm/hyp/switch.c | 24 +++++++++++++++++++
> > 2 files changed, 46 insertions(+), 35 deletions(-)
> >
> > diff --git a/arch/arm64/kvm/hyp/entry.S b/arch/arm64/kvm/hyp/entry.S
> > index e41a161..40f349b 100644
> > --- a/arch/arm64/kvm/hyp/entry.S
> > +++ b/arch/arm64/kvm/hyp/entry.S
> > @@ -172,40 +172,27 @@ ENTRY(__fpsimd_guest_restore)
> > // x1: vcpu
> > // x2-x29,lr: vcpu regs
> > // vcpu x0-x1 on the stack
> > - stp x2, x3, [sp, #-16]!
> > - stp x4, lr, [sp, #-16]!
> > -
> > -alternative_if_not ARM64_HAS_VIRT_HOST_EXTN
> > - mrs x2, cptr_el2
> > - bic x2, x2, #CPTR_EL2_TFP
> > - msr cptr_el2, x2
> > -alternative_else
> > - mrs x2, cpacr_el1
> > - orr x2, x2, #CPACR_EL1_FPEN
> > - msr cpacr_el1, x2
> > -alternative_endif
> > - isb
> > -
> > - mov x3, x1
> > -
> > - ldr x0, [x3, #VCPU_HOST_CONTEXT]
> > - kern_hyp_va x0
> > - add x0, x0, #CPU_GP_REG_OFFSET(CPU_FP_REGS)
> > - bl __fpsimd_save_state
> > -
> > - add x2, x3, #VCPU_CONTEXT
> > - add x0, x2, #CPU_GP_REG_OFFSET(CPU_FP_REGS)
> > - bl __fpsimd_restore_state
> > -
> > - // Skip restoring fpexc32 for AArch64 guests
> > - mrs x1, hcr_el2
> > - tbnz x1, #HCR_RW_SHIFT, 1f
> > - ldr x4, [x3, #VCPU_FPEXC32_EL2]
> > - msr fpexc32_el2, x4
> > -1:
> > - ldp x4, lr, [sp], #16
> > - ldp x2, x3, [sp], #16
> > - ldp x0, x1, [sp], #16
> > -
> > + stp x2, x3, [sp, #-144]!
> > + stp x4, x5, [sp, #16]
> > + stp x6, x7, [sp, #32]
> > + stp x8, x9, [sp, #48]
> > + stp x10, x11, [sp, #64]
> > + stp x12, x13, [sp, #80]
> > + stp x14, x15, [sp, #96]
> > + stp x16, x17, [sp, #112]
> > + stp x18, lr, [sp, #128]
> > +
> > + bl __hyp_switch_fpsimd
> > +
> > + ldp x4, x5, [sp, #16]
> > + ldp x6, x7, [sp, #32]
> > + ldp x8, x9, [sp, #48]
> > + ldp x10, x11, [sp, #64]
> > + ldp x12, x13, [sp, #80]
> > + ldp x14, x15, [sp, #96]
> > + ldp x16, x17, [sp, #112]
> > + ldp x18, lr, [sp, #128]
> > + ldp x0, x1, [sp, #144]
> > + ldp x2, x3, [sp], #160
> > eret
> > ENDPROC(__fpsimd_guest_restore)
> > diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
> > index d964523..c0796c4 100644
> > --- a/arch/arm64/kvm/hyp/switch.c
> > +++ b/arch/arm64/kvm/hyp/switch.c
> > @@ -318,6 +318,30 @@ static bool __hyp_text __skip_instr(struct kvm_vcpu *vcpu)
> > }
> > }
> >
> > +void __hyp_text __hyp_switch_fpsimd(u64 esr __always_unused,
> > + struct kvm_vcpu *vcpu)
> > +{
> > + kvm_cpu_context_t *host_ctxt;
> > +
> > + if (has_vhe())
> > + write_sysreg(read_sysreg(cpacr_el1) | CPACR_EL1_FPEN,
> > + cpacr_el1);
> > + else
> > + write_sysreg(read_sysreg(cptr_el2) & ~(u64)CPTR_EL2_TFP,
> > + cptr_el2);
>
> Is there no way to do alternative() in C or does it always come down to
> different inline asms?
>
has_vhe() should resolve to a static key, and I prefer this over the
previous alternative construct we had for selecting function calls in C,
as that resultet in having to follow too many levels of indirection.
Thanks,
-Christoffer
^ permalink raw reply
* [PATCH v10 04/18] KVM: arm/arm64: Introduce kvm_arch_vcpu_run_pid_change
From: Christoffer Dall @ 2018-05-24 8:11 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180523144026.GM13470@e103592.cambridge.arm.com>
On Wed, May 23, 2018 at 03:40:26PM +0100, Dave Martin wrote:
> On Wed, May 23, 2018 at 03:34:20PM +0100, Alex Benn?e wrote:
> >
> > Dave Martin <Dave.Martin@arm.com> writes:
> >
> > > From: Christoffer Dall <christoffer.dall@linaro.org>
> > >
> > > KVM/ARM differs from other architectures in having to maintain an
> > > additional virtual address space from that of the host and the
> > > guest, because we split the execution of KVM across both EL1 and
> > > EL2.
> > >
> > > This results in a need to explicitly map data structures into EL2
> > > (hyp) which are accessed from the hyp code. As we are about to be
> > > more clever with our FPSIMD handling on arm64, which stores data in
> > > the task struct and uses thread_info flags, we will have to map
> > > parts of the currently executing task struct into the EL2 virtual
> > > address space.
> > >
> > > However, we don't want to do this on every KVM_RUN, because it is a
> > > fairly expensive operation to walk the page tables, and the common
> > > execution mode is to map a single thread to a VCPU. By introducing
> > > a hook that architectures can select with
> > > HAVE_KVM_VCPU_RUN_PID_CHANGE, we do not introduce overhead for
> > > other architectures, but have a simple way to only map the data we
> > > need when required for arm64.
> > >
> > > This patch introduces the framework only, and wires it up in the
> > > arm/arm64 KVM common code.
> > >
> > > No functional change.
> > >
> > > Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
> > > Signed-off-by: Dave Martin <Dave.Martin@arm.com>
> > > Reviewed-by: Marc Zyngier <marc.zyngier@arm.com>
> > > ---
> > > include/linux/kvm_host.h | 9 +++++++++
> > > virt/kvm/Kconfig | 3 +++
> > > virt/kvm/kvm_main.c | 7 ++++++-
> > > 3 files changed, 18 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> > > index 6930c63..4268ace 100644
> > > --- a/include/linux/kvm_host.h
> > > +++ b/include/linux/kvm_host.h
> > > @@ -1276,4 +1276,13 @@ static inline long kvm_arch_vcpu_async_ioctl(struct file *filp,
> > > void kvm_arch_mmu_notifier_invalidate_range(struct kvm *kvm,
> > > unsigned long start, unsigned long end);
> > >
> > > +#ifdef CONFIG_HAVE_KVM_VCPU_RUN_PID_CHANGE
> > > +int kvm_arch_vcpu_run_pid_change(struct kvm_vcpu *vcpu);
> > > +#else
> > > +static inline int kvm_arch_vcpu_run_pid_change(struct kvm_vcpu *vcpu)
> > > +{
> > > + return 0;
> > > +}
> > > +#endif /* CONFIG_HAVE_KVM_VCPU_RUN_PID_CHANGE */
> > > +
> > > #endif
> > > diff --git a/virt/kvm/Kconfig b/virt/kvm/Kconfig
> > > index cca7e06..72143cf 100644
> > > --- a/virt/kvm/Kconfig
> > > +++ b/virt/kvm/Kconfig
> > > @@ -54,3 +54,6 @@ config HAVE_KVM_IRQ_BYPASS
> > >
> > > config HAVE_KVM_VCPU_ASYNC_IOCTL
> > > bool
> > > +
> > > +config HAVE_KVM_VCPU_RUN_PID_CHANGE
> > > + bool
> >
> > This almost threw me as I thought you might be able to enable this and
> > break the build, but apparently not:
> >
> > Reviewed-by: Alex Benn?e <alex.bennee@linaro.org>
>
> Without a "help", the option seems non-interactive and cannot be true
> unless something selects it. It seems a bit weird to me too, but the
> idiom appears widely used...
>
Indeed, I've copied this idiom from other things before and nobody has
complained, so I think it works (without any further deep insights into
the inner workings of Kconfig).
Thanks,
-Christoffer
^ permalink raw reply
* Patch "staging: bcm2835-audio: Release resources on module_exit()" has been added to the 4.16-stable tree
From: gregkh at linuxfoundation.org @ 2018-05-24 8:10 UTC (permalink / raw)
To: linux-arm-kernel
This is a note to let you know that I've just added the patch titled
staging: bcm2835-audio: Release resources on module_exit()
to the 4.16-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
The filename of the patch is:
staging-bcm2835-audio-release-resources-on-module_exit.patch
and it can be found in the queue-4.16 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@vger.kernel.org> know about it.
>From foo at baz Thu May 24 10:04:42 CEST 2018
From: Kirill Marinushkin <k.marinushkin@gmail.com>
Date: Fri, 23 Mar 2018 20:32:54 +0100
Subject: staging: bcm2835-audio: Release resources on module_exit()
From: Kirill Marinushkin <k.marinushkin@gmail.com>
[ Upstream commit 626118b472d2eb45f83a0276a18d3e6a01c69f6a ]
In the current implementation, `rmmod snd_bcm2835` does not release
resources properly. It causes an oops when trying to list sound devices.
This commit fixes it.
The details WRT allocation / free are described below.
Device structure WRT allocation:
pdev
\childdev[]
\card
\chip
\pcm
\ctl
Allocation / register sequence:
* childdev: devm_kzalloc - freed during driver detach
* childdev: device_initialize - freed during device_unregister
* pdev: devres_alloc - freed during driver detach
* childdev: device_add - removed during device_unregister
* pdev, childdev: devres_add - freed during driver detach
* card: snd_card_new - freed during snd_card_free
* chip: kzalloc - freed during kfree
* card, chip: snd_device_new - freed during snd_device_free
* chip: new_pcm - TODO: free pcm
* chip: new_ctl - TODO: free ctl
* card: snd_card_register - unregistered during snd_card_free
Free / unregister sequence:
* card: snd_card_free
* card, chip: snd_device_free
* childdev: device_unregister
* chip: kfree
Steps to reproduce the issue before this commit:
~~~~
$ rmmod snd_bcm2835
$ aplay -L
[ 138.648130] Unable to handle kernel paging request at virtual address 7f1343c0
[ 138.660415] pgd = ad8f0000
[ 138.665567] [7f1343c0] *pgd=3864c811, *pte=00000000, *ppte=00000000
[ 138.674887] Internal error: Oops: 7 [#1] SMP ARM
[ 138.683571] Modules linked in: sha256_generic cfg80211 rfkill snd_pcm snd_timer
snd fixed uio_pdrv_genirq uio ip_tables x_tables ipv6 [last unloaded: snd_bcm2835
]
[ 138.706594] CPU: 3 PID: 463 Comm: aplay Tainted: G WC 4.15.0-rc1-v
7+ #6
[ 138.719833] Hardware name: BCM2835
[ 138.726016] task: b877ac00 task.stack: aebec000
[ 138.733408] PC is at try_module_get+0x38/0x24c
[ 138.740813] LR is at snd_ctl_open+0x58/0x194 [snd]
[ 138.748485] pc : [<801c4d5c>] lr : [<7f0e6b2c>] psr: 20000013
[ 138.757709] sp : aebedd60 ip : aebedd88 fp : aebedd84
[ 138.765884] r10: 00000000 r9 : 00000004 r8 : 7f0ed440
[ 138.774040] r7 : b7e469b0 r6 : 7f0e6b2c r5 : afd91900 r4 : 7f1343c0
[ 138.783571] r3 : aebec000 r2 : 00000001 r1 : b877ac00 r0 : 7f1343c0
[ 138.793084] Flags: nzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user
[ 138.803300] Control: 10c5387d Table: 2d8f006a DAC: 00000055
[ 138.812064] Process aplay (pid: 463, stack limit = 0xaebec210)
[ 138.820868] Stack: (0xaebedd60 to 0xaebee000)
[ 138.828207] dd60: 00000000 b848d000 afd91900 00000000 b7e469b0 7f0ed440 aebedda4 aebedd88
[ 138.842371] dd80: 7f0e6b2c 801c4d30 afd91900 7f0ea4dc 00000000 b7e469b0 aebeddcc aebedda8
[ 138.856611] dda0: 7f0e250c 7f0e6ae0 7f0e2464 b8478ec0 b7e469b0 afd91900 7f0ea388 00000000
[ 138.870864] ddc0: aebeddf4 aebeddd0 802ce590 7f0e2470 8090ab64 afd91900 afd91900 b7e469b0
[ 138.885301] dde0: afd91908 802ce4e4 aebede1c aebeddf8 802c57b4 802ce4f0 afd91900 aebedea8
[ 138.900110] de00: b7fa4c00 00000000 00000000 00000004 aebede3c aebede20 802c6ba8 802c56b4
[ 138.915260] de20: aebedea8 00000000 aebedf5c 00000000 aebedea4 aebede40 802d9a68 802c6b58
[ 138.930661] de40: b874ddd0 00000000 00000000 00000001 00000041 00000000 afd91900 aebede70
[ 138.946402] de60: 00000000 00000000 00000002 b7e469b0 b8a87610 b8d6ab80 801852f8 00080000
[ 138.962314] de80: aebedf5c aebedea8 00000001 80108464 aebec000 00000000 aebedf4c aebedea8
[ 138.978414] dea0: 802dacd4 802d970c b8a87610 b8d6ab80 a7982bc6 00000009 af363019 b9231480
[ 138.994617] dec0: 00000000 b8c038a0 b7e469b0 00000101 00000002 00000238 00000000 00000000
[ 139.010823] dee0: 00000000 aebedee8 00080000 0000000f aebedf3c aebedf00 802ed7e4 80843f94
[ 139.027025] df00: 00000003 00080000 b9231490 b9231480 00000000 00080000 af363000 00000000
[ 139.043229] df20: 00000005 00000002 ffffff9c 00000000 00080000 ffffff9c af363000 00000003
[ 139.059430] df40: aebedf94 aebedf50 802c6f70 802dac70 aebec000 00000000 00000001 00000000
[ 139.075629] df60: 00020000 00000004 00000100 00000001 7ebe577c 0002e038 00000000 00000005
[ 139.091828] df80: 80108464 aebec000 aebedfa4 aebedf98 802c7060 802c6e6c 00000000 aebedfa8
[ 139.108025] dfa0: 801082c0 802c7040 7ebe577c 0002e038 7ebe577c 00080000 00000b98 e81c8400
[ 139.124222] dfc0: 7ebe577c 0002e038 00000000 00000005 7ebe57e4 00a20af8 7ebe57f0 76f87394
[ 139.140419] dfe0: 00000000 7ebe55c4 76ec88e8 76df1d9c 60000010 7ebe577c 00000000 00000000
[ 139.156715] [<801c4d5c>] (try_module_get) from [<7f0e6b2c>] (snd_ctl_open+0x58/0x194 [snd])
[ 139.173222] [<7f0e6b2c>] (snd_ctl_open [snd]) from [<7f0e250c>] (snd_open+0xa8/0x14c [snd])
[ 139.189683] [<7f0e250c>] (snd_open [snd]) from [<802ce590>] (chrdev_open+0xac/0x188)
[ 139.205465] [<802ce590>] (chrdev_open) from [<802c57b4>] (do_dentry_open+0x10c/0x314)
[ 139.221347] [<802c57b4>] (do_dentry_open) from [<802c6ba8>] (vfs_open+0x5c/0x88)
[ 139.236788] [<802c6ba8>] (vfs_open) from [<802d9a68>] (path_openat+0x368/0x944)
[ 139.248270] [<802d9a68>] (path_openat) from [<802dacd4>] (do_filp_open+0x70/0xc4)
[ 139.263731] [<802dacd4>] (do_filp_open) from [<802c6f70>] (do_sys_open+0x110/0x1d4)
[ 139.279378] [<802c6f70>] (do_sys_open) from [<802c7060>] (SyS_open+0x2c/0x30)
[ 139.290647] [<802c7060>] (SyS_open) from [<801082c0>] (ret_fast_syscall+0x0/0x28)
[ 139.306021] Code: e3c3303f e5932004 e2822001 e5832004 (e5943000)
[ 139.316265] ---[ end trace 7f3f7f6193b663ed ]---
[ 139.324956] note: aplay[463] exited with preempt_count 1
~~~~
Signed-off-by: Kirill Marinushkin <k.marinushkin@gmail.com>
Cc: Eric Anholt <eric@anholt.net>
Cc: Stefan Wahren <stefan.wahren@i2se.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Florian Fainelli <f.fainelli@gmail.com>
Cc: Ray Jui <rjui@broadcom.com>
Cc: Scott Branden <sbranden@broadcom.com>
Cc: bcm-kernel-feedback-list at broadcom.com
Cc: Michael Zoran <mzoran@crowfest.net>
Cc: Andy Shevchenko <andy.shevchenko@gmail.com>
Cc: linux-rpi-kernel at lists.infradead.org
Cc: linux-arm-kernel at lists.infradead.org
Cc: devel at driverdev.osuosl.org
Cc: linux-kernel at vger.kernel.org
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Sasha Levin <alexander.levin@microsoft.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
drivers/staging/vc04_services/bcm2835-audio/bcm2835.c | 54 ++++++++----------
1 file changed, 25 insertions(+), 29 deletions(-)
--- a/drivers/staging/vc04_services/bcm2835-audio/bcm2835.c
+++ b/drivers/staging/vc04_services/bcm2835-audio/bcm2835.c
@@ -25,6 +25,10 @@ MODULE_PARM_DESC(enable_compat_alsa,
static void snd_devm_unregister_child(struct device *dev, void *res)
{
struct device *childdev = *(struct device **)res;
+ struct bcm2835_chip *chip = dev_get_drvdata(childdev);
+ struct snd_card *card = chip->card;
+
+ snd_card_free(card);
device_unregister(childdev);
}
@@ -50,6 +54,13 @@ static int snd_devm_add_child(struct dev
return 0;
}
+static void snd_bcm2835_release(struct device *dev)
+{
+ struct bcm2835_chip *chip = dev_get_drvdata(dev);
+
+ kfree(chip);
+}
+
static struct device *
snd_create_device(struct device *parent,
struct device_driver *driver,
@@ -65,6 +76,7 @@ snd_create_device(struct device *parent,
device_initialize(device);
device->parent = parent;
device->driver = driver;
+ device->release = snd_bcm2835_release;
dev_set_name(device, "%s", name);
@@ -75,18 +87,19 @@ snd_create_device(struct device *parent,
return device;
}
-static int snd_bcm2835_free(struct bcm2835_chip *chip)
-{
- kfree(chip);
- return 0;
-}
-
/* component-destructor
* (see "Management of Cards and Components")
*/
static int snd_bcm2835_dev_free(struct snd_device *device)
{
- return snd_bcm2835_free(device->device_data);
+ struct bcm2835_chip *chip = device->device_data;
+ struct snd_card *card = chip->card;
+
+ /* TODO: free pcm, ctl */
+
+ snd_device_free(card, chip);
+
+ return 0;
}
/* chip-specific constructor
@@ -111,7 +124,7 @@ static int snd_bcm2835_create(struct snd
err = snd_device_new(card, SNDRV_DEV_LOWLEVEL, chip, &ops);
if (err) {
- snd_bcm2835_free(chip);
+ kfree(chip);
return err;
}
@@ -119,31 +132,14 @@ static int snd_bcm2835_create(struct snd
return 0;
}
-static void snd_devm_card_free(struct device *dev, void *res)
+static struct snd_card *snd_bcm2835_card_new(struct device *dev)
{
- struct snd_card *snd_card = *(struct snd_card **)res;
-
- snd_card_free(snd_card);
-}
-
-static struct snd_card *snd_devm_card_new(struct device *dev)
-{
- struct snd_card **dr;
struct snd_card *card;
int ret;
- dr = devres_alloc(snd_devm_card_free, sizeof(*dr), GFP_KERNEL);
- if (!dr)
- return ERR_PTR(-ENOMEM);
-
ret = snd_card_new(dev, -1, NULL, THIS_MODULE, 0, &card);
- if (ret) {
- devres_free(dr);
+ if (ret)
return ERR_PTR(ret);
- }
-
- *dr = card;
- devres_add(dev, dr);
return card;
}
@@ -260,7 +256,7 @@ static int snd_add_child_device(struct d
return PTR_ERR(child);
}
- card = snd_devm_card_new(child);
+ card = snd_bcm2835_card_new(child);
if (IS_ERR(card)) {
dev_err(child, "Failed to create card");
return PTR_ERR(card);
@@ -302,7 +298,7 @@ static int snd_add_child_device(struct d
return err;
}
- dev_set_drvdata(child, card);
+ dev_set_drvdata(child, chip);
dev_info(child, "card created with %d channels\n", numchans);
return 0;
Patches currently in stable-queue which might be from k.marinushkin at gmail.com are
queue-4.16/staging-bcm2835-audio-release-resources-on-module_exit.patch
^ permalink raw reply
* [PATCH v5 4/4] gpiolib: discourage gpiochip_add_pin[group]_range for DT pinctrls
From: Linus Walleij @ 2018-05-24 8:07 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <2969992a1005d7e7f4ea23d4c684d2f10d4a0aca.1526935804.git.chunkeey@gmail.com>
On Mon, May 21, 2018 at 10:57 PM, Christian Lamparter
<chunkeey@gmail.com> wrote:
> This patch adds the stern warning to the kerneldoc text of both
> gpiochip_add_pin[group]_range() functions in hope of detering
> developers from ever using them in their DeviceTree-supported
> pinctrl drivers in the future.
>
> For anyone affected: Please refer to Section 2.1 of
> Documentation/devicetree/bindings/gpio/gpio.txt on how to
> bind pinctrl and gpio drivers via the "gpio-ranges" property.
>
> Signed-off-by: Christian Lamparter <chunkeey@gmail.com>
I'm a big fan of this patch, so applied.
I wouldn't mind a pr_info() warning about it at runtime too,
but this is nice.
Yours,
Linus Walleij
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox