* Re: [PATCH v6 5/5] mailbox: Add support for i.MX7D messaging unit
From: Oleksij Rempel @ 2018-07-23 19:11 UTC (permalink / raw)
To: Lucas Stach
Cc: Mark Rutland, A.s. Dong, devicetree, Rob Herring, dl-linux-imx,
kernel, Fabio Estevam, Shawn Guo, Vladimir Zapolskiy,
linux-arm-kernel
In-Reply-To: <1532366380.3163.109.camel@pengutronix.de>
[-- Attachment #1.1: Type: text/plain, Size: 12269 bytes --]
On Mon, Jul 23, 2018 at 07:19:40PM +0200, Lucas Stach wrote:
> Am Sonntag, den 22.07.2018, 08:39 +0200 schrieb Oleksij Rempel:
> > The Mailbox controller is able to send messages (up to 4 32 bit words)
> > between the endpoints.
> >
> > This driver was tested using the mailbox-test driver sending messages
> > between the Cortex-A7 and the Cortex-M4.
> >
> > > Reviewed-by: Dong Aisheng <aisheng.dong@nxp.com>
> > > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> > ---
> > drivers/mailbox/Kconfig | 6 +
> > drivers/mailbox/Makefile | 2 +
> > drivers/mailbox/imx-mailbox.c | 273 ++++++++++++++++++++++++++++++++++
> > 3 files changed, 281 insertions(+)
> > create mode 100644 drivers/mailbox/imx-mailbox.c
> >
> > diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig
> > index a2bb27446dce..79060ddc380d 100644
> > --- a/drivers/mailbox/Kconfig
> > +++ b/drivers/mailbox/Kconfig
> > @@ -15,6 +15,12 @@ config ARM_MHU
> > > The controller has 3 mailbox channels, the last of which can be
> > > used in Secure mode only.
> >
> > +config IMX_MBOX
> > > + tristate "i.MX Mailbox"
> > > + depends on ARCH_MXC || COMPILE_TEST
> > > + help
> > > + Mailbox implementation for i.MX Messaging Unit (MU).
> > +
> > config PLATFORM_MHU
> > > tristate "Platform MHU Mailbox"
> > > depends on OF
> > diff --git a/drivers/mailbox/Makefile b/drivers/mailbox/Makefile
> > index cc23c3a43fcd..ba2fe1b6dd62 100644
> > --- a/drivers/mailbox/Makefile
> > +++ b/drivers/mailbox/Makefile
> > > @@ -7,6 +7,8 @@ obj-$(CONFIG_MAILBOX_TEST) += mailbox-test.o
> >
> > > obj-$(CONFIG_ARM_MHU) += arm_mhu.o
> >
> > > +obj-$(CONFIG_IMX_MBOX) += imx-mailbox.o
> > +
> > > obj-$(CONFIG_PLATFORM_MHU) += platform_mhu.o
> >
> > > obj-$(CONFIG_PL320_MBOX) += pl320-ipc.o
> > diff --git a/drivers/mailbox/imx-mailbox.c b/drivers/mailbox/imx-mailbox.c
> > new file mode 100644
> > index 000000000000..29cf2876db01
> > --- /dev/null
> > +++ b/drivers/mailbox/imx-mailbox.c
> > @@ -0,0 +1,273 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (c) 2018 Pengutronix, Oleksij Rempel <o.rempel@pengutronix.de>
> > + */
> > +
> > +#include <linux/clk.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/io.h>
> > +#include <linux/kernel.h>
> > +#include <linux/mailbox_controller.h>
> > +#include <linux/module.h>
> > +#include <linux/of_device.h>
> > +
> > +/* Transmit Register */
> > > +#define IMX_MU_xTRn(x) (0x00 + 4 * (x))
> > +/* Receive Register */
> > > +#define IMX_MU_xRRn(x) (0x10 + 4 * (x))
> > +/* Status Register */
> > > +#define IMX_MU_xSR 0x20
> > > +#define IMX_MU_xSR_TEn(x) BIT(20 + (3 - (x)))
> > > +#define IMX_MU_xSR_RFn(x) BIT(24 + (3 - (x)))
> > > +#define IMX_MU_xSR_BRDIP BIT(9)
> > +
> > +/* Control Register */
> > > +#define IMX_MU_xCR 0x24
> > +/* Transmit Interrupt Enable */
> > > +#define IMX_MU_xCR_TIEn(x) BIT(20 + (3 - (x)))
> > +/* Receive Interrupt Enable */
> > > +#define IMX_MU_xCR_RIEn(x) BIT(24 + (3 - (x)))
> > +
> > > +#define IMX_MU_CHANS 4u
> > +
> > +struct imx_mu_con_priv {
> > > > + int irq;
> > > > + unsigned int idx;
> > > > + char *irq_desc;
> > +};
> > +
> > +struct imx_mu_priv {
> > > > + struct device *dev;
> > > > + void __iomem *base;
> > +
> > > > + struct mbox_controller mbox;
> > > > + struct mbox_chan mbox_chans[IMX_MU_CHANS];
> > +
> > > + struct imx_mu_con_priv con_priv[IMX_MU_CHANS];
> > > > + struct clk *clk;
> > +
> > > > + bool side_b;
> > +};
> > +
> > +static struct imx_mu_priv *to_imx_mu_priv(struct mbox_controller *mbox)
> > +{
> > > + return container_of(mbox, struct imx_mu_priv, mbox);
> > +}
> > +
> > +static void imx_mu_write(struct imx_mu_priv *priv, u32 val, u32 offs)
> > +{
> > + iowrite32(val, priv->base + offs);
>
> This driver is never going to be used on a device with port based IO,
> so iowrite doesn't make much sense here, just use writel. Same comment
> applies to the below read function.
I read this and do not understand what is actual problem:
https://lwn.net/Articles/102232/
"By default, these functions are simply wrappers around readb() and friends.
The explicit pointer type for the argument will generate warnings, however,
if a driver passes in an integer type."
So, is it wrong or not? If yes, why?
> Also, given that those functions are not really shortening the code in
> the user they may also be removed completely IMHO.
Is it subjective or technical issue?
> > +}
> > +
> > +static u32 imx_mu_read(struct imx_mu_priv *priv, u32 offs)
> > +{
> > > + return ioread32(priv->base + offs);
> > +}
> > +
> > +static u32 imx_mu_rmw(struct imx_mu_priv *priv, u32 offs, u32 set, u32 clr)
> > +{
> > > + u32 val;
> > +
> > > + val = imx_mu_read(priv, offs);
> > > + val &= ~clr;
> > > + val |= set;
> > > + imx_mu_write(priv, val, offs);
> > +
> > > + return val;
> > +}
> > +
> > +static irqreturn_t imx_mu_isr(int irq, void *p)
> > +{
> > > + struct mbox_chan *chan = p;
> > > + struct imx_mu_priv *priv = to_imx_mu_priv(chan->mbox);
> > > + struct imx_mu_con_priv *cp = chan->con_priv;
> > > + u32 val, ctrl, dat;
> > +
> > > + ctrl = imx_mu_read(priv, IMX_MU_xCR);
> > > + val = imx_mu_read(priv, IMX_MU_xSR);
> > > + val &= IMX_MU_xSR_TEn(cp->idx) | IMX_MU_xSR_RFn(cp->idx);
> > > + val &= ctrl & (IMX_MU_xCR_TIEn(cp->idx) | IMX_MU_xCR_RIEn(cp->idx));
> > > + if (!val)
> > > + return IRQ_NONE;
> > +
> > > + if (val & IMX_MU_xSR_TEn(cp->idx)) {
> > > + imx_mu_rmw(priv, IMX_MU_xCR, 0, IMX_MU_xCR_TIEn(cp->idx));
> > > + mbox_chan_txdone(chan, 0);
> > > + }
> > +
> > > + if (val & IMX_MU_xSR_RFn(cp->idx)) {
> > > + dat = imx_mu_read(priv, IMX_MU_xRRn(cp->idx));
> > > + mbox_chan_received_data(chan, (void *)&dat);
> > > + }
> > +
> > > + return IRQ_HANDLED;
> > +}
> > +
> > +static bool imx_mu_last_tx_done(struct mbox_chan *chan)
> > +{
> > > + struct imx_mu_priv *priv = to_imx_mu_priv(chan->mbox);
> > > + struct imx_mu_con_priv *cp = chan->con_priv;
> > > + u32 val;
> > +
> > > + val = imx_mu_read(priv, IMX_MU_xSR);
> > > + /* test if transmit register is empty */
> > + return val & IMX_MU_xSR_TEn(cp->idx);
>
> I guess
> "return imx_mu_read(priv, IMX_MU_xSR) & IMX_MU_xSR_TEn(cp->idx);" is
> shorter and equally well understood.
Same as before:
Is it subjective or technical issue?
> > +}
> > +
> > +static int imx_mu_send_data(struct mbox_chan *chan, void *data)
> > +{
> > > + struct imx_mu_priv *priv = to_imx_mu_priv(chan->mbox);
> > > + struct imx_mu_con_priv *cp = chan->con_priv;
> > > + u32 *arg = data;
> > +
> > > + if (!imx_mu_last_tx_done(chan))
> > > + return -EBUSY;
> > +
> > > + imx_mu_write(priv, *arg, IMX_MU_xTRn(cp->idx));
> > + imx_mu_rmw(priv, IMX_MU_xCR, IMX_MU_xSR_TEn(cp->idx), 0);
>
> In multi-channel mode this RMW cycle needs some kind of locking. As
> this register is also changed from the irq handler, this probably needs
> to be a irqsave spinlock.
ok.
> > +
> > > + return 0;
> > +}
> > +
> > +static int imx_mu_startup(struct mbox_chan *chan)
> > +{
> > > + struct imx_mu_priv *priv = to_imx_mu_priv(chan->mbox);
> > > + struct imx_mu_con_priv *cp = chan->con_priv;
> > > + int ret;
> > +
> > > + cp->irq_desc = devm_kasprintf(priv->dev, GFP_KERNEL, "imx_mu_chan[%i]",
> > > + cp->idx);
> > > + if (!cp->irq_desc)
> > > + return -ENOMEM;
> > +
> > > + ret = devm_request_irq(priv->dev, cp->irq, imx_mu_isr,
> > + IRQF_SHARED, cp->irq_desc, chan);
>
> Using the devm_ variants of those functions doesn't make sense when the
> resources aren't tied to the device lifetime. As you are tearing them
> down manually in imx_mu_shutdown anyways, just use the raw variants of
> those functions.
So, it is not a problem and there is no advantage. Right?
> > + if (ret) {
> > > + dev_err(priv->dev,
> > > + "Unable to acquire IRQ %d\n", cp->irq);
> > > + return ret;
> > > + }
> > +
> > > + imx_mu_rmw(priv, IMX_MU_xCR, IMX_MU_xCR_RIEn(cp->idx), 0);
> > +
> > > + return 0;
> > +}
> > +
> > +static void imx_mu_shutdown(struct mbox_chan *chan)
> > +{
> > > + struct imx_mu_priv *priv = to_imx_mu_priv(chan->mbox);
> > > + struct imx_mu_con_priv *cp = chan->con_priv;
> > +
> > > + imx_mu_rmw(priv, IMX_MU_xCR, 0,
> > > + IMX_MU_xCR_TIEn(cp->idx) | IMX_MU_xCR_RIEn(cp->idx));
> > +
> > > + devm_free_irq(priv->dev, cp->irq, chan);
> > > + devm_kfree(priv->dev, cp->irq_desc);
> > +}
> > +
> > +static const struct mbox_chan_ops imx_mu_ops = {
> > > + .send_data = imx_mu_send_data,
> > > + .startup = imx_mu_startup,
> > > + .shutdown = imx_mu_shutdown,
> > +};
> > +
> > +static void imx_mu_init_generic(struct imx_mu_priv *priv)
> > +{
> > > + if (priv->side_b)
> > > + return;
> > +
> > > + /* Set default MU configuration */
> > > + imx_mu_write(priv, 0, IMX_MU_xCR);
> > +}
> > +
> > +static int imx_mu_probe(struct platform_device *pdev)
> > +{
> > > + struct device *dev = &pdev->dev;
> > > + struct device_node *np = dev->of_node;
> > > + struct resource *iomem;
> > > + struct imx_mu_priv *priv;
> > > + unsigned int i;
> > > + int irq, ret;
> > +
> > > + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> > > + if (!priv)
> > > + return -ENOMEM;
> > +
> > > + priv->dev = dev;
> > +
> > > + iomem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > > + priv->base = devm_ioremap_resource(&pdev->dev, iomem);
> > > + if (IS_ERR(priv->base))
> > > + return PTR_ERR(priv->base);
> > +
> > > + irq = platform_get_irq(pdev, 0);
> > > + if (irq < 0)
> > > + return irq;
> > +
> > > + priv->clk = devm_clk_get(dev, NULL);
> > > + if (IS_ERR(priv->clk)) {
> > > + if (PTR_ERR(priv->clk) != -ENOENT)
> > > + return PTR_ERR(priv->clk);
> > +
> > > + priv->clk = NULL;
> > > + }
> > +
> > > + ret = clk_prepare_enable(priv->clk);
> > > + if (ret) {
> > > + dev_err(dev, "Failed to enable clock\n");
> > > + return ret;
> > > + }
> > +
> > > + for (i = 0; i < IMX_MU_CHANS; i++) {
> > > + struct imx_mu_con_priv *cp = &priv->con_priv[i];
> > +
> > > + cp->idx = i;
> > > + cp->irq = irq;
> > > + priv->mbox_chans[i].con_priv = cp;
> > > + }
> > +
> > > + if (of_property_read_bool(np, "fsl,mu-side-b"))
> > + priv->side_b = true;
>
> No need for the if clause here. Just assign the return value from
> of_property_read_bool to priv->side_b.
Not against kernel style and not broken.
Is it subjective or technical issue? I assume I like more red color...
> > +
> > > + priv->mbox.dev = dev;
> > > + priv->mbox.ops = &imx_mu_ops;
> > > + priv->mbox.chans = priv->mbox_chans;
> > > + priv->mbox.num_chans = IMX_MU_CHANS;
> > > + priv->mbox.txdone_irq = true;
> > +
> > > + platform_set_drvdata(pdev, priv);
> > +
> > > + imx_mu_init_generic(priv);
> > +
> > > + return mbox_controller_register(&priv->mbox);
> > +}
> > +
> > +static int imx_mu_remove(struct platform_device *pdev)
> > +{
> > > + struct imx_mu_priv *priv = platform_get_drvdata(pdev);
> > +
> > > + mbox_controller_unregister(&priv->mbox);
> > > + clk_disable_unprepare(priv->clk);
> > +
> > > + return 0;
> > +}
> > +
> > +static const struct of_device_id imx_mu_dt_ids[] = {
> > > + { .compatible = "fsl,imx6sx-mu" },
> > > + { },
> > +};
> > +MODULE_DEVICE_TABLE(of, imx_mu_dt_ids);
> > +
> > +static struct platform_driver imx_mu_driver = {
> > > > + .probe = imx_mu_probe,
> > > > + .remove = imx_mu_remove,
> > > + .driver = {
> > > > + .name = "imx_mu",
> > > + .of_match_table = imx_mu_dt_ids,
> > > + },
> > +};
> > +module_platform_driver(imx_mu_driver);
> > +
> > > +MODULE_AUTHOR("Oleksij Rempel <o.rempel@pengutronix.de>");
> > +MODULE_DESCRIPTION("Message Unit driver for i.MX");
> > +MODULE_LICENSE("GPL v2");
>
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 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: 488 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
* [PATCH v6 5/5] mailbox: Add support for i.MX7D messaging unit
From: Oleksij Rempel @ 2018-07-23 19:11 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1532366380.3163.109.camel@pengutronix.de>
On Mon, Jul 23, 2018 at 07:19:40PM +0200, Lucas Stach wrote:
> Am Sonntag, den 22.07.2018, 08:39 +0200 schrieb Oleksij Rempel:
> > The Mailbox controller is able to send messages (up to 4 32 bit words)
> > between the endpoints.
> >
> > This driver was tested using the mailbox-test driver sending messages
> > between the Cortex-A7 and the Cortex-M4.
> >
> > > Reviewed-by: Dong Aisheng <aisheng.dong@nxp.com>
> > > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> > ---
> > ?drivers/mailbox/Kconfig???????|???6 +
> > ?drivers/mailbox/Makefile??????|???2 +
> > ?drivers/mailbox/imx-mailbox.c | 273 ++++++++++++++++++++++++++++++++++
> > ?3 files changed, 281 insertions(+)
> > ?create mode 100644 drivers/mailbox/imx-mailbox.c
> >
> > diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig
> > index a2bb27446dce..79060ddc380d 100644
> > --- a/drivers/mailbox/Kconfig
> > +++ b/drivers/mailbox/Kconfig
> > @@ -15,6 +15,12 @@ config ARM_MHU
> > > ? ??The controller has 3 mailbox channels, the last of which can be
> > > ? ??used in Secure mode only.
> > ?
> > +config IMX_MBOX
> > > + tristate "i.MX Mailbox"
> > > + depends on ARCH_MXC || COMPILE_TEST
> > > + help
> > > + ??Mailbox implementation for i.MX Messaging Unit (MU).
> > +
> > ?config PLATFORM_MHU
> > > ? tristate "Platform MHU Mailbox"
> > > ? depends on OF
> > diff --git a/drivers/mailbox/Makefile b/drivers/mailbox/Makefile
> > index cc23c3a43fcd..ba2fe1b6dd62 100644
> > --- a/drivers/mailbox/Makefile
> > +++ b/drivers/mailbox/Makefile
> > > @@ -7,6 +7,8 @@ obj-$(CONFIG_MAILBOX_TEST) += mailbox-test.o
> > ?
> > > ?obj-$(CONFIG_ARM_MHU) += arm_mhu.o
> > ?
> > > +obj-$(CONFIG_IMX_MBOX) += imx-mailbox.o
> > +
> > > ?obj-$(CONFIG_PLATFORM_MHU) += platform_mhu.o
> > ?
> > > ?obj-$(CONFIG_PL320_MBOX) += pl320-ipc.o
> > diff --git a/drivers/mailbox/imx-mailbox.c b/drivers/mailbox/imx-mailbox.c
> > new file mode 100644
> > index 000000000000..29cf2876db01
> > --- /dev/null
> > +++ b/drivers/mailbox/imx-mailbox.c
> > @@ -0,0 +1,273 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (c) 2018 Pengutronix, Oleksij Rempel <o.rempel@pengutronix.de>
> > + */
> > +
> > +#include <linux/clk.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/io.h>
> > +#include <linux/kernel.h>
> > +#include <linux/mailbox_controller.h>
> > +#include <linux/module.h>
> > +#include <linux/of_device.h>
> > +
> > +/* Transmit Register */
> > > +#define IMX_MU_xTRn(x) (0x00 + 4 * (x))
> > +/* Receive Register */
> > > +#define IMX_MU_xRRn(x) (0x10 + 4 * (x))
> > +/* Status Register */
> > > +#define IMX_MU_xSR 0x20
> > > +#define IMX_MU_xSR_TEn(x) BIT(20 + (3 - (x)))
> > > +#define IMX_MU_xSR_RFn(x) BIT(24 + (3 - (x)))
> > > +#define IMX_MU_xSR_BRDIP BIT(9)
> > +
> > +/* Control Register */
> > > +#define IMX_MU_xCR 0x24
> > +/* Transmit Interrupt Enable */
> > > +#define IMX_MU_xCR_TIEn(x) BIT(20 + (3 - (x)))
> > +/* Receive Interrupt Enable */
> > > +#define IMX_MU_xCR_RIEn(x) BIT(24 + (3 - (x)))
> > +
> > > +#define IMX_MU_CHANS 4u
> > +
> > +struct imx_mu_con_priv {
> > > > + int irq;
> > > > + unsigned int idx;
> > > > + char *irq_desc;
> > +};
> > +
> > +struct imx_mu_priv {
> > > > + struct device *dev;
> > > > + void __iomem *base;
> > +
> > > > + struct mbox_controller mbox;
> > > > + struct mbox_chan mbox_chans[IMX_MU_CHANS];
> > +
> > > + struct imx_mu_con_priv??con_priv[IMX_MU_CHANS];
> > > > + struct clk *clk;
> > +
> > > > + bool side_b;
> > +};
> > +
> > +static struct imx_mu_priv *to_imx_mu_priv(struct mbox_controller *mbox)
> > +{
> > > + return container_of(mbox, struct imx_mu_priv, mbox);
> > +}
> > +
> > +static void imx_mu_write(struct imx_mu_priv *priv, u32 val, u32 offs)
> > +{
> > + iowrite32(val, priv->base + offs);
>
> This driver is never going to be used on a device with port based IO,
> so iowrite doesn't make much sense here, just use writel. Same comment
> applies to the below read function.
I read this and do not understand what is actual problem:
https://lwn.net/Articles/102232/
"By default, these functions are simply wrappers around readb() and friends.
The explicit pointer type for the argument will generate warnings, however,
if a driver passes in an integer type."
So, is it wrong or not? If yes, why?
> Also, given that those functions are not really shortening the code in
> the user they may also be removed completely IMHO.
Is it subjective or technical issue?
> > +}
> > +
> > +static u32 imx_mu_read(struct imx_mu_priv *priv, u32 offs)
> > +{
> > > + return ioread32(priv->base + offs);
> > +}
> > +
> > +static u32 imx_mu_rmw(struct imx_mu_priv *priv, u32 offs, u32 set, u32 clr)
> > +{
> > > + u32 val;
> > +
> > > + val = imx_mu_read(priv, offs);
> > > + val &= ~clr;
> > > + val |= set;
> > > + imx_mu_write(priv, val, offs);
> > +
> > > + return val;
> > +}
> > +
> > +static irqreturn_t imx_mu_isr(int irq, void *p)
> > +{
> > > + struct mbox_chan *chan = p;
> > > + struct imx_mu_priv *priv = to_imx_mu_priv(chan->mbox);
> > > + struct imx_mu_con_priv *cp = chan->con_priv;
> > > + u32 val, ctrl, dat;
> > +
> > > + ctrl = imx_mu_read(priv, IMX_MU_xCR);
> > > + val = imx_mu_read(priv, IMX_MU_xSR);
> > > + val &= IMX_MU_xSR_TEn(cp->idx) | IMX_MU_xSR_RFn(cp->idx);
> > > + val &= ctrl & (IMX_MU_xCR_TIEn(cp->idx) | IMX_MU_xCR_RIEn(cp->idx));
> > > + if (!val)
> > > + return IRQ_NONE;
> > +
> > > + if (val & IMX_MU_xSR_TEn(cp->idx)) {
> > > + imx_mu_rmw(priv, IMX_MU_xCR, 0, IMX_MU_xCR_TIEn(cp->idx));
> > > + mbox_chan_txdone(chan, 0);
> > > + }
> > +
> > > + if (val & IMX_MU_xSR_RFn(cp->idx)) {
> > > + dat = imx_mu_read(priv, IMX_MU_xRRn(cp->idx));
> > > + mbox_chan_received_data(chan, (void *)&dat);
> > > + }
> > +
> > > + return IRQ_HANDLED;
> > +}
> > +
> > +static bool imx_mu_last_tx_done(struct mbox_chan *chan)
> > +{
> > > + struct imx_mu_priv *priv = to_imx_mu_priv(chan->mbox);
> > > + struct imx_mu_con_priv *cp = chan->con_priv;
> > > + u32 val;
> > +
> > > + val = imx_mu_read(priv, IMX_MU_xSR);
> > > + /* test if transmit register is empty */
> > + return val & IMX_MU_xSR_TEn(cp->idx);
>
> I guess
> "return imx_mu_read(priv, IMX_MU_xSR) & IMX_MU_xSR_TEn(cp->idx);" is
> shorter and equally well understood.
Same as before:
Is it subjective or technical issue?
> > +}
> > +
> > +static int imx_mu_send_data(struct mbox_chan *chan, void *data)
> > +{
> > > + struct imx_mu_priv *priv = to_imx_mu_priv(chan->mbox);
> > > + struct imx_mu_con_priv *cp = chan->con_priv;
> > > + u32 *arg = data;
> > +
> > > + if (!imx_mu_last_tx_done(chan))
> > > + return -EBUSY;
> > +
> > > + imx_mu_write(priv, *arg, IMX_MU_xTRn(cp->idx));
> > + imx_mu_rmw(priv, IMX_MU_xCR, IMX_MU_xSR_TEn(cp->idx), 0);
>
> In multi-channel mode this RMW cycle needs some kind of locking. As
> this register is also changed from the irq handler, this probably needs
> to be a irqsave spinlock.
ok.
> > +
> > > + return 0;
> > +}
> > +
> > +static int imx_mu_startup(struct mbox_chan *chan)
> > +{
> > > + struct imx_mu_priv *priv = to_imx_mu_priv(chan->mbox);
> > > + struct imx_mu_con_priv *cp = chan->con_priv;
> > > + int ret;
> > +
> > > + cp->irq_desc = devm_kasprintf(priv->dev, GFP_KERNEL, "imx_mu_chan[%i]",
> > > + ??????cp->idx);
> > > + if (!cp->irq_desc)
> > > + return -ENOMEM;
> > +
> > > + ret = devm_request_irq(priv->dev, cp->irq, imx_mu_isr,
> > + ???????IRQF_SHARED, cp->irq_desc, chan);
>
> Using the devm_ variants of those functions doesn't make sense when the
> resources aren't tied to the device lifetime. As you are tearing them
> down manually in imx_mu_shutdown anyways, just use the raw variants of
> those functions.
So, it is not a problem and there is no advantage. Right?
> > + if (ret) {
> > > + dev_err(priv->dev,
> > > + "Unable to acquire IRQ %d\n", cp->irq);
> > > + return ret;
> > > + }
> > +
> > > + imx_mu_rmw(priv, IMX_MU_xCR, IMX_MU_xCR_RIEn(cp->idx), 0);
> > +
> > > + return 0;
> > +}
> > +
> > +static void imx_mu_shutdown(struct mbox_chan *chan)
> > +{
> > > + struct imx_mu_priv *priv = to_imx_mu_priv(chan->mbox);
> > > + struct imx_mu_con_priv *cp = chan->con_priv;
> > +
> > > + imx_mu_rmw(priv, IMX_MU_xCR, 0,
> > > + ???IMX_MU_xCR_TIEn(cp->idx) | IMX_MU_xCR_RIEn(cp->idx));
> > +
> > > + devm_free_irq(priv->dev, cp->irq, chan);
> > > + devm_kfree(priv->dev, cp->irq_desc);
> > +}
> > +
> > +static const struct mbox_chan_ops imx_mu_ops = {
> > > + .send_data = imx_mu_send_data,
> > > + .startup = imx_mu_startup,
> > > + .shutdown = imx_mu_shutdown,
> > +};
> > +
> > +static void imx_mu_init_generic(struct imx_mu_priv *priv)
> > +{
> > > + if (priv->side_b)
> > > + return;
> > +
> > > + /* Set default MU configuration */
> > > + imx_mu_write(priv, 0, IMX_MU_xCR);
> > +}
> > +
> > +static int imx_mu_probe(struct platform_device *pdev)
> > +{
> > > + struct device *dev = &pdev->dev;
> > > + struct device_node *np = dev->of_node;
> > > + struct resource *iomem;
> > > + struct imx_mu_priv *priv;
> > > + unsigned int i;
> > > + int irq, ret;
> > +
> > > + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> > > + if (!priv)
> > > + return -ENOMEM;
> > +
> > > + priv->dev = dev;
> > +
> > > + iomem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > > + priv->base = devm_ioremap_resource(&pdev->dev, iomem);
> > > + if (IS_ERR(priv->base))
> > > + return PTR_ERR(priv->base);
> > +
> > > + irq = platform_get_irq(pdev, 0);
> > > + if (irq < 0)
> > > + return irq;
> > +
> > > + priv->clk = devm_clk_get(dev, NULL);
> > > + if (IS_ERR(priv->clk)) {
> > > + if (PTR_ERR(priv->clk) != -ENOENT)
> > > + return PTR_ERR(priv->clk);
> > +
> > > + priv->clk = NULL;
> > > + }
> > +
> > > + ret = clk_prepare_enable(priv->clk);
> > > + if (ret) {
> > > + dev_err(dev, "Failed to enable clock\n");
> > > + return ret;
> > > + }
> > +
> > > + for (i = 0; i < IMX_MU_CHANS; i++) {
> > > + struct imx_mu_con_priv *cp = &priv->con_priv[i];
> > +
> > > + cp->idx = i;
> > > + cp->irq = irq;
> > > + priv->mbox_chans[i].con_priv = cp;
> > > + }
> > +
> > > + if (of_property_read_bool(np, "fsl,mu-side-b"))
> > + priv->side_b = true;
>
> No need for the if clause here. Just assign the return value from
> of_property_read_bool to priv->side_b.
Not against kernel style and not broken.
Is it subjective or technical issue? I assume I like more red color...
> > +
> > > + priv->mbox.dev = dev;
> > > + priv->mbox.ops = &imx_mu_ops;
> > > + priv->mbox.chans = priv->mbox_chans;
> > > + priv->mbox.num_chans = IMX_MU_CHANS;
> > > + priv->mbox.txdone_irq = true;
> > +
> > > + platform_set_drvdata(pdev, priv);
> > +
> > > + imx_mu_init_generic(priv);
> > +
> > > + return mbox_controller_register(&priv->mbox);
> > +}
> > +
> > +static int imx_mu_remove(struct platform_device *pdev)
> > +{
> > > + struct imx_mu_priv *priv = platform_get_drvdata(pdev);
> > +
> > > + mbox_controller_unregister(&priv->mbox);
> > > + clk_disable_unprepare(priv->clk);
> > +
> > > + return 0;
> > +}
> > +
> > +static const struct of_device_id imx_mu_dt_ids[] = {
> > > + { .compatible = "fsl,imx6sx-mu" },
> > > + { },
> > +};
> > +MODULE_DEVICE_TABLE(of, imx_mu_dt_ids);
> > +
> > +static struct platform_driver imx_mu_driver = {
> > > > + .probe = imx_mu_probe,
> > > > + .remove = imx_mu_remove,
> > > + .driver = {
> > > > + .name = "imx_mu",
> > > + .of_match_table = imx_mu_dt_ids,
> > > + },
> > +};
> > +module_platform_driver(imx_mu_driver);
> > +
> > > +MODULE_AUTHOR("Oleksij Rempel <o.rempel@pengutronix.de>");
> > +MODULE_DESCRIPTION("Message Unit driver for i.MX");
> > +MODULE_LICENSE("GPL v2");
>
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180723/06b2537f/attachment.sig>
^ permalink raw reply
* Applied "regulator: pfuze100: add optional disable switch-regulators binding" to the regulator tree
From: Mark Brown @ 2018-07-23 19:11 UTC (permalink / raw)
To: Marco Felsch; +Cc: Mark Brown, linux-kernel
The patch
regulator: pfuze100: add optional disable switch-regulators binding
has been applied to the regulator tree at
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark
From 78170811a2048de8e77a27a053be8b3eb3d4e556 Mon Sep 17 00:00:00 2001
From: Marco Felsch <m.felsch@pengutronix.de>
Date: Mon, 23 Jul 2018 09:47:46 +0200
Subject: [PATCH] regulator: pfuze100: add optional disable switch-regulators
binding
This binding is used to keep the backward compatibility with the current
dtb's [1]. The binding informs the driver that the unused switch regulators
can be disabled.
If it is not specified, the driver doesn't disable the switch regulators.
[1] https://patchwork.kernel.org/patch/10490381/
Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
Documentation/devicetree/bindings/regulator/pfuze100.txt | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/Documentation/devicetree/bindings/regulator/pfuze100.txt b/Documentation/devicetree/bindings/regulator/pfuze100.txt
index 672c939045ff..c7610718adff 100644
--- a/Documentation/devicetree/bindings/regulator/pfuze100.txt
+++ b/Documentation/devicetree/bindings/regulator/pfuze100.txt
@@ -4,6 +4,15 @@ Required properties:
- compatible: "fsl,pfuze100", "fsl,pfuze200", "fsl,pfuze3000", "fsl,pfuze3001"
- reg: I2C slave address
+Optional properties:
+- fsl,pfuze-support-disable-sw: Boolean, if present disable all unused switch
+ regulators to save power consumption. Attention, ensure that all important
+ regulators (e.g. DDR ref, DDR supply) has set the "regulator-always-on"
+ property. If not present, the switched regualtors are always on and can't be
+ disabled. This binding is a workaround to keep backward compatibility with
+ old dtb's which rely on the fact that the switched regulators are always on
+ and don't mark them explicit as "regulator-always-on".
+
Required child node:
- regulators: This is the list of child nodes that specify the regulator
initialization data for defined regulators. Please refer to below doc
--
2.18.0
^ permalink raw reply related
* Re: Does /dev/urandom now block until initialised ?
From: Theodore Y. Ts'o @ 2018-07-23 19:11 UTC (permalink / raw)
To: Jeffrey Walton; +Cc: Ken Moffat, Linux Crypto Mailing List, lkml
In-Reply-To: <CAH8yC8kT=OBjxX8ye9GYec5Hs2NORtPtf8e6DPxP8JiUH9BNyg@mail.gmail.com>
On Mon, Jul 23, 2018 at 12:11:12PM -0400, Jeffrey Walton wrote:
>
> I believe Stephan Mueller wrote up the weakness a couple of years ago.
> He's the one who explained the interactions to me. Mueller was even
> cited at https://github.com/systemd/systemd/issues/4167.
Stephan had a lot of complaints about the existing random driver.
That's because he has a replacement driver that he has been pushing,
and instead of giving explicit complaints with specific patches to fix
those specific issues, he have a generalized blast of complaints, plus
a "big bang rewrite".
I've reviewed his lrng doc, and this specific issue was not among his
complaints. Quite a while ago, I had gone through his document, and
had specifically addressed each of his complaints. As far as I have
been able determine, all of the specific technical complaints (as
opposed to personal preference issues) have been addressed.
His complaint is a text book complaint about how *not* to file a bug
report. That being said, we try to take bug reports from as many
sources as possible even if they aren't well formed or submitted in
the ideal place.
(I'm reminded of Linux's networking scalability limitations which
Microsoft filed in the Wall Street Journal 15+ years ago --- and which
only applied if you had 4 CPU's and four 10 megabit networking cards;
if you had four CPU's and a 100 megabit networking card, Linux would
grind Microsoft into the dust; still it was a bug, and we appreciated
the report and we fixed it, even if it wasn't filed in the ideal
forum. :-)
> It is too bad he Mueller not receive credit for it in the CVE database.
As near as I can tell, he doesn't deserve it for this particular
issue. It's all Jann Horn and Google's Project Zero. (And his
writeup is a textbook example of how to report this sort of issue with
great specifity and analysis.)
- Ted
^ permalink raw reply
* Applied "regulator: pfuze100: add support to en-/disable switch regulators" to the regulator tree
From: Mark Brown @ 2018-07-23 19:11 UTC (permalink / raw)
To: Marco Felsch; +Cc: Mark Brown, linux-kernel
The patch
regulator: pfuze100: add support to en-/disable switch regulators
has been applied to the regulator tree at
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark
From 9d2fd4f0ddfbc4aa1135000df34caebc02793a26 Mon Sep 17 00:00:00 2001
From: Marco Felsch <m.felsch@pengutronix.de>
Date: Mon, 23 Jul 2018 09:47:47 +0200
Subject: [PATCH] regulator: pfuze100: add support to en-/disable switch
regulators
Add enable/disable support for switch regulators on pfuze100.
Based on commit 5fe156f1cab4 ("regulator: pfuze100: add enable/disable for
switch") which is reverted due to boot regressions by commit 464a5686e6c9
("regulator: Revert "regulator: pfuze100: add enable/disable for switch"").
Disabling the switch regulators will only be done if the user specifies
"fsl,pfuze-support-disable-sw" in its device tree to keep backward
compatibility with current dtb's [1].
[1] https://patchwork.kernel.org/patch/10490381/
Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
drivers/regulator/pfuze100-regulator.c | 36 ++++++++++++++++++++++++++
1 file changed, 36 insertions(+)
diff --git a/drivers/regulator/pfuze100-regulator.c b/drivers/regulator/pfuze100-regulator.c
index cde6eda1d283..31c3a236120a 100644
--- a/drivers/regulator/pfuze100-regulator.c
+++ b/drivers/regulator/pfuze100-regulator.c
@@ -17,6 +17,8 @@
#include <linux/slab.h>
#include <linux/regmap.h>
+#define PFUZE_FLAG_DISABLE_SW BIT(1)
+
#define PFUZE_NUMREGS 128
#define PFUZE100_VOL_OFFSET 0
#define PFUZE100_STANDBY_OFFSET 1
@@ -50,10 +52,12 @@ struct pfuze_regulator {
struct regulator_desc desc;
unsigned char stby_reg;
unsigned char stby_mask;
+ bool sw_reg;
};
struct pfuze_chip {
int chip_id;
+ int flags;
struct regmap *regmap;
struct device *dev;
struct pfuze_regulator regulator_descs[PFUZE100_MAX_REGULATOR];
@@ -170,6 +174,17 @@ static const struct regulator_ops pfuze100_sw_regulator_ops = {
.set_ramp_delay = pfuze100_set_ramp_delay,
};
+static const struct regulator_ops pfuze100_sw_disable_regulator_ops = {
+ .enable = regulator_enable_regmap,
+ .disable = regulator_disable_regmap,
+ .is_enabled = regulator_is_enabled_regmap,
+ .list_voltage = regulator_list_voltage_linear,
+ .set_voltage_sel = regulator_set_voltage_sel_regmap,
+ .get_voltage_sel = regulator_get_voltage_sel_regmap,
+ .set_voltage_time_sel = regulator_set_voltage_time_sel,
+ .set_ramp_delay = pfuze100_set_ramp_delay,
+};
+
static const struct regulator_ops pfuze100_swb_regulator_ops = {
.enable = regulator_enable_regmap,
.disable = regulator_disable_regmap,
@@ -209,9 +224,12 @@ static const struct regulator_ops pfuze100_swb_regulator_ops = {
.uV_step = (step), \
.vsel_reg = (base) + PFUZE100_VOL_OFFSET, \
.vsel_mask = 0x3f, \
+ .enable_reg = (base) + PFUZE100_MODE_OFFSET, \
+ .enable_mask = 0xf, \
}, \
.stby_reg = (base) + PFUZE100_STANDBY_OFFSET, \
.stby_mask = 0x3f, \
+ .sw_reg = true, \
}
#define PFUZE100_SWB_REG(_chip, _name, base, mask, voltages) \
@@ -471,6 +489,9 @@ static int pfuze_parse_regulators_dt(struct pfuze_chip *chip)
if (!np)
return -EINVAL;
+ if (of_property_read_bool(np, "fsl,pfuze-support-disable-sw"))
+ chip->flags |= PFUZE_FLAG_DISABLE_SW;
+
parent = of_get_child_by_name(np, "regulators");
if (!parent) {
dev_err(dev, "regulators node not found\n");
@@ -703,6 +724,21 @@ static int pfuze100_regulator_probe(struct i2c_client *client,
}
}
+ /*
+ * Allow SW regulators to turn off. Checking it trough a flag is
+ * a workaround to keep the backward compatibility with existing
+ * old dtb's which may relay on the fact that we didn't disable
+ * the switched regulator till yet.
+ */
+ if (pfuze_chip->flags & PFUZE_FLAG_DISABLE_SW) {
+ if (pfuze_chip->regulator_descs[i].sw_reg) {
+ desc->ops = &pfuze100_sw_disable_regulator_ops;
+ desc->enable_val = 0x8;
+ desc->disable_val = 0x0;
+ desc->enable_time = 500;
+ }
+ }
+
config.dev = &client->dev;
config.init_data = init_data;
config.driver_data = pfuze_chip;
--
2.18.0
^ permalink raw reply related
* Applied "ASoC: rockchip-i2s: add description for px30" to the asoc tree
From: Mark Brown @ 2018-07-23 19:11 UTC (permalink / raw)
To: Liang Chen; +Cc: Rob Herring, alsa-devel, Mark Brown
The patch
ASoC: rockchip-i2s: add description for px30
has been applied to the asoc tree at
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark
>From 2f562a4739602d96928de0b040ce46b00d8d3adc Mon Sep 17 00:00:00 2001
From: Liang Chen <cl@rock-chips.com>
Date: Mon, 23 Jul 2018 17:25:21 +0800
Subject: [PATCH] ASoC: rockchip-i2s: add description for px30
Add "rockchip,px30-i2s", "rockchip,rk3066-i2s" for i2s on px30 platform.
Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Liang Chen <cl@rock-chips.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
Documentation/devicetree/bindings/sound/rockchip-i2s.txt | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/sound/rockchip-i2s.txt b/Documentation/devicetree/bindings/sound/rockchip-i2s.txt
index b208a752576c..54aefab71f2c 100644
--- a/Documentation/devicetree/bindings/sound/rockchip-i2s.txt
+++ b/Documentation/devicetree/bindings/sound/rockchip-i2s.txt
@@ -7,6 +7,7 @@ Required properties:
- compatible: should be one of the following:
- "rockchip,rk3066-i2s": for rk3066
+ - "rockchip,px30-i2s", "rockchip,rk3066-i2s": for px30
- "rockchip,rk3036-i2s", "rockchip,rk3066-i2s": for rk3036
- "rockchip,rk3188-i2s", "rockchip,rk3066-i2s": for rk3188
- "rockchip,rk3228-i2s", "rockchip,rk3066-i2s": for rk3228
--
2.18.0
^ permalink raw reply related
* [PATCH 0/5] sh_eth: clean up the TSU register accessors
From: Sergei Shtylyov @ 2018-07-23 18:08 UTC (permalink / raw)
To: netdev, David S. Miller; +Cc: linux-renesas-soc
Hello!
Here's a set of 5 patches against DaveM's 'net-next.git' repo. They do a final
clean up of the TSU register accessors...
[1/1] sh_eth: uninline sh_eth_tsu_get_offset()
[2/3] sh_eth: make sh_eth_tsu_get_offset() match its name
[3/3] sh_eth: call sh_eth_tsu_get_offset() from TSU register accessors
[4/5] sh_eth: make sh_eth_tsu_write_entry() take 'offset' parameter
[5/5] sh_eth: make sh_eth_tsu_{read|write}_entry() prototypes symmetric
MBR, Sergei
^ permalink raw reply
* Re: [PATCH v2 3/6] dt-bindings: sound: rockchip-i2s: add description for px30
From: Mark Brown @ 2018-07-23 19:10 UTC (permalink / raw)
To: cl
Cc: heiko, robh+dt, mark.rutland, klaus.goger, hjc, jagan, djw,
jacob-chen, devicetree, linux-arm-kernel, linux-rockchip,
linux-kernel, ulf.hansson, linux-mmc, frank.wang, lgirdwood,
alsa-devel, gregkh, catalin.marinas, will.deacon, yamada.masahiro,
arnd, zhangqing, shawn.lin, kever.yang, david.wu, huangtao,
tony.xie, sugar.zhang, huibin.hong, william.wu, sandy.huang
In-Reply-To: <1532337924-4185-4-git-send-email-cl@rock-chips.com>
[-- Attachment #1: Type: text/plain, Size: 480 bytes --]
On Mon, Jul 23, 2018 at 05:25:21PM +0800, cl@rock-chips.com wrote:
> From: Liang Chen <cl@rock-chips.com>
>
> Add "rockchip,px30-i2s", "rockchip,rk3066-i2s" for i2s on px30 platform.
Please submit patches using subject lines reflecting the style for the
subsystem. This makes it easier for people to identify relevant
patches. Look at what existing commits in the area you're changing are
doing and make sure your subject lines visually resemble what they're
doing.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH 0/1] t7406: avoid failures solely due to timing issues
From: Junio C Hamano @ 2018-07-23 19:10 UTC (permalink / raw)
To: Torsten Bögershausen; +Cc: git, Johannes Schindelin via GitGitGadget
In-Reply-To: <pull.12.git.gitgitgadget@gmail.com>
"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
writes:
> This fixes a regression test that produces false positives occasionally: https://git-for-windows.visualstudio.com/git/_build/results?buildId=14035&view=logs
>
[jc: forwrding to Torsten for <208b2ede-4833-f062-16f2-f35b8a8ce099@web.de>]
> Johannes Schindelin (1):
> t7406: avoid failures solely due to timing issues
>
> t/t7406-submodule-update.sh | 11 ++++++-----
> 1 file changed, 6 insertions(+), 5 deletions(-)
>
>
> base-commit: b7bd9486b055c3f967a870311e704e3bb0654e4f
> Published-As: https://github.com/gitgitgadget/git/releases/tags/pr-12%2Fdscho%2Fmore-robust-t7406-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-12/dscho/more-robust-t7406-v1
> Pull-Request: https://github.com/gitgitgadget/git/pull/12
^ permalink raw reply
* [Intel-wired-lan] [jkirsher-next-queue:dev-queue] BUILD SUCCESS 9d936faa44ae56a07e93b7bbbb2c1df9d923a2ba
From: kbuild test robot @ 2018-07-23 19:10 UTC (permalink / raw)
To: intel-wired-lan
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/next-queue.git dev-queue
branch HEAD: 9d936faa44ae56a07e93b7bbbb2c1df9d923a2ba ice: Change struct members from bool to u8
elapsed time: 26m
configs tested: 95
The following configs have been built successfully.
More configs may be tested in the coming days.
x86_64 acpi-redef
x86_64 allyesdebian
x86_64 nfsroot
mips lasat_defconfig
parisc a500_defconfig
i386 tinyconfig
i386 randconfig-n0-201829
x86_64 randconfig-x007-201829
x86_64 randconfig-x003-201829
x86_64 randconfig-x000-201829
x86_64 randconfig-x005-201829
x86_64 randconfig-x004-201829
x86_64 randconfig-x008-201829
x86_64 randconfig-x001-201829
x86_64 randconfig-x009-201829
x86_64 randconfig-x002-201829
x86_64 randconfig-x006-201829
i386 randconfig-i0-201829
i386 randconfig-i1-201829
x86_64 randconfig-v0-07240215
i386 randconfig-a0-201829
i386 randconfig-a1-201829
alpha defconfig
parisc allnoconfig
parisc b180_defconfig
parisc c3000_defconfig
parisc defconfig
x86_64 randconfig-x018-201829
x86_64 randconfig-x011-201829
x86_64 randconfig-x015-201829
x86_64 randconfig-x019-201829
x86_64 randconfig-x014-201829
x86_64 randconfig-x010-201829
x86_64 randconfig-x013-201829
x86_64 randconfig-x016-201829
x86_64 randconfig-x017-201829
x86_64 randconfig-x012-201829
i386 randconfig-s0-201829
i386 randconfig-s1-201829
powerpc pcm030_defconfig
powerpc tqm8548_defconfig
sh hp6xx_defconfig
microblaze mmu_defconfig
microblaze nommu_defconfig
sparc defconfig
sparc64 allnoconfig
sparc64 defconfig
c6x evmc6678_defconfig
h8300 h8300h-sim_defconfig
nios2 10m50_defconfig
xtensa common_defconfig
xtensa iss_defconfig
i386 randconfig-x012-201829
i386 randconfig-x017-201829
i386 randconfig-x014-201829
i386 randconfig-x016-201829
i386 randconfig-x013-201829
i386 randconfig-x011-201829
i386 randconfig-x018-201829
i386 randconfig-x010-201829
i386 randconfig-x015-201829
i386 randconfig-x019-201829
m68k m5475evb_defconfig
m68k multi_defconfig
m68k sun3_defconfig
sh allnoconfig
sh rsk7269_defconfig
sh sh7785lcr_32bit_defconfig
sh titan_defconfig
i386 randconfig-x008-201829
i386 randconfig-x009-201829
i386 randconfig-x005-201829
i386 randconfig-x000-201829
i386 randconfig-x003-201829
i386 randconfig-x001-201829
i386 randconfig-x004-201829
i386 randconfig-x006-201829
i386 randconfig-x007-201829
i386 randconfig-x002-201829
i386 randconfig-x078-201829
i386 randconfig-x070-201829
i386 randconfig-x075-201829
i386 randconfig-x076-201829
i386 randconfig-x074-201829
i386 randconfig-x079-201829
i386 randconfig-x071-201829
i386 randconfig-x073-201829
i386 randconfig-x072-201829
i386 randconfig-x077-201829
ia64 alldefconfig
ia64 allnoconfig
ia64 defconfig
openrisc or1ksim_defconfig
um i386_defconfig
um x86_64_defconfig
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
^ permalink raw reply
* [PATCH v2 3/6] dt-bindings: sound: rockchip-i2s: add description for px30
From: Mark Brown @ 2018-07-23 19:10 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1532337924-4185-4-git-send-email-cl@rock-chips.com>
On Mon, Jul 23, 2018 at 05:25:21PM +0800, cl at rock-chips.com wrote:
> From: Liang Chen <cl@rock-chips.com>
>
> Add "rockchip,px30-i2s", "rockchip,rk3066-i2s" for i2s on px30 platform.
Please submit patches using subject lines reflecting the style for the
subsystem. This makes it easier for people to identify relevant
patches. Look at what existing commits in the area you're changing are
doing and make sure your subject lines visually resemble what they're
doing.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180723/991cc840/attachment.sig>
^ permalink raw reply
* Re: [PATCH v2 3/6] dt-bindings: sound: rockchip-i2s: add description for px30
From: Mark Brown @ 2018-07-23 19:10 UTC (permalink / raw)
To: cl
Cc: mark.rutland, ulf.hansson, heiko, huibin.hong, catalin.marinas,
shawn.lin, will.deacon, alsa-devel, hjc, yamada.masahiro,
tony.xie, klaus.goger, sugar.zhang, linux-rockchip, jacob-chen,
jagan, huangtao, devicetree, arnd, frank.wang, zhangqing,
kever.yang, robh+dt, djw, sandy.huang, linux-arm-kernel, gregkh,
linux-mmc, lgirdwood, linux-kernel, david.wu, william.wu
In-Reply-To: <1532337924-4185-4-git-send-email-cl@rock-chips.com>
[-- Attachment #1.1: Type: text/plain, Size: 480 bytes --]
On Mon, Jul 23, 2018 at 05:25:21PM +0800, cl@rock-chips.com wrote:
> From: Liang Chen <cl@rock-chips.com>
>
> Add "rockchip,px30-i2s", "rockchip,rk3066-i2s" for i2s on px30 platform.
Please submit patches using subject lines reflecting the style for the
subsystem. This makes it easier for people to identify relevant
patches. Look at what existing commits in the area you're changing are
doing and make sure your subject lines visually resemble what they're
doing.
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 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
* Re: rte_mbuf library likely()/unlikely()
From: Morten Brørup @ 2018-07-23 19:09 UTC (permalink / raw)
To: Honnappa Nagarahalli, Olivier Matz; +Cc: dev
In-Reply-To: <HE1PR0801MB1930747F372E5B543B4F273098560@HE1PR0801MB1930.eurprd08.prod.outlook.com>
I haven't performance tested, but they are compiler branch prediction hints pointing out the most likely execution path, so I expect them to have a positive effect.
E.g. the first comparison in __rte_pktmbuf_read() is very unlikely to be true - it would mean that the application is trying to read data beyond the packet.
Please also refer to:
https://cellperformance.beyond3d.com/articles/2006/04/branch-patterns-using-gcc.html
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Honnappa Nagarahalli
> Sent: Monday, July 23, 2018 7:52 PM
> To: Morten Brørup; Olivier Matz
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] rte_mbuf library likely()/unlikely()
>
> Do you see any performance improvements with these changes?
>
> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Morten Brørup
> Sent: Monday, July 23, 2018 8:54 AM
> To: Olivier Matz <olivier.matz@6wind.com>
> Cc: dev@dpdk.org
> Subject: [dpdk-dev] rte_mbuf library likely()/unlikely()
>
> Hi Olivier,
>
>
>
> I noticed that __rte_pktmbuf_read() could do with an unlikely(), so I went
> through the entire library. Here are my suggested modifications.
>
^ permalink raw reply
* [PATCH] defaultsetup.conf: Enable security flags+pie by default
From: Khem Raj @ 2018-07-23 19:09 UTC (permalink / raw)
To: openembedded-core
This has been an opt-in for so long, some distributions e.g.
poky-lsb uses it by default however, since most of linux
distros have started to default to these settings for security
enhancements, time has come for OE to make it default too
Signed-off-by: Khem Raj <raj.khem@gmail.com>
---
meta/conf/distro/defaultsetup.conf | 1 +
1 file changed, 1 insertion(+)
diff --git a/meta/conf/distro/defaultsetup.conf b/meta/conf/distro/defaultsetup.conf
index ca2f9178d2..352e279596 100644
--- a/meta/conf/distro/defaultsetup.conf
+++ b/meta/conf/distro/defaultsetup.conf
@@ -1,6 +1,7 @@
include conf/distro/include/default-providers.inc
include conf/distro/include/default-versions.inc
include conf/distro/include/default-distrovars.inc
+require conf/distro/include/security_flags.inc
include conf/distro/include/world-broken.inc
TCMODE ?= "default"
--
2.18.0
^ permalink raw reply related
* Re: Reg. rpm changelog
From: Alexander Kanavin @ 2018-07-23 19:09 UTC (permalink / raw)
To: Burton, Ross; +Cc: Yocto-mailing-list
In-Reply-To: <CAJTo0LawohE+FR0g9+igixjt3QYXcRT+A-ij_3ohxr_jSLmwdg@mail.gmail.com>
Note that we programmatically create the rpm spec files, and there is
currently no support for writing changelogs into them.
Generally, hand-crafted changelogs are just a maintenance burden, if you ask me.
Alex
2018-07-23 20:49 GMT+02:00 Burton, Ross <ross.burton@intel.com>:
> On 23 July 2018 at 18:30, Vikram Chhibber <vikram.chhibber@gmail.com> wrote:
>> Thanks Mark. Is it possible to somehow use already created changelog while
>> building rpm?
>
> You mean that you'll maintain an independent changelog but want it to
> be in the RPM?
>
> Ross
> --
> _______________________________________________
> yocto mailing list
> yocto@yoctoproject.org
> https://lists.yoctoproject.org/listinfo/yocto
^ permalink raw reply
* [PATCH 9/9] tcmu: use u64 for dev_size
From: Mike Christie @ 2018-07-23 19:07 UTC (permalink / raw)
To: target-devel
We use unsigned long, size_t and u64 for dev_size. This has us
standardize on u64.
Signed-off-by: Mike Christie <mchristi@redhat.com>
---
drivers/target/target_core_user.c | 21 ++++++++-------------
1 file changed, 8 insertions(+), 13 deletions(-)
diff --git a/drivers/target/target_core_user.c b/drivers/target/target_core_user.c
index cfe4f4b..b34179a 100644
--- a/drivers/target/target_core_user.c
+++ b/drivers/target/target_core_user.c
@@ -137,7 +137,7 @@ struct tcmu_dev {
struct inode *inode;
struct tcmu_mailbox *mb_addr;
- size_t dev_size;
+ uint64_t dev_size;
u32 cmdr_size;
u32 cmdr_last_cleaned;
/* Offset of data area from start of mb */
@@ -2016,7 +2016,7 @@ enum {
static match_table_t tokens = {
{Opt_dev_config, "dev_config=%s"},
- {Opt_dev_size, "dev_size=%u"},
+ {Opt_dev_size, "dev_size=%s"},
{Opt_hw_block_size, "hw_block_size=%d"},
{Opt_hw_max_sectors, "hw_max_sectors=%d"},
{Opt_nl_reply_supported, "nl_reply_supported=%d"},
@@ -2083,7 +2083,7 @@ static ssize_t tcmu_set_configfs_dev_params(struct se_device *dev,
const char *page, ssize_t count)
{
struct tcmu_dev *udev = TCMU_DEV(dev);
- char *orig, *ptr, *opts, *arg_p;
+ char *orig, *ptr, *opts;
substring_t args[MAX_OPT_ARGS];
int ret = 0, token;
@@ -2108,15 +2108,10 @@ static ssize_t tcmu_set_configfs_dev_params(struct se_device *dev,
pr_debug("TCMU: Referencing Path: %s\n", udev->dev_config);
break;
case Opt_dev_size:
- arg_p = match_strdup(&args[0]);
- if (!arg_p) {
- ret = -ENOMEM;
- break;
- }
- ret = kstrtoul(arg_p, 0, (unsigned long *) &udev->dev_size);
- kfree(arg_p);
+ ret = match_u64(&args[0], &udev->dev_size);
if (ret < 0)
- pr_err("kstrtoul() failed for dev_size=\n");
+ pr_err("match_u64() failed for dev_size=. Error %d.\n",
+ ret);
break;
case Opt_hw_block_size:
ret = tcmu_set_dev_attrib(&args[0],
@@ -2154,7 +2149,7 @@ static ssize_t tcmu_show_configfs_dev_params(struct se_device *dev, char *b)
bl = sprintf(b + bl, "Config: %s ",
udev->dev_config[0] ? udev->dev_config : "NULL");
- bl += sprintf(b + bl, "Size: %zu ", udev->dev_size);
+ bl += sprintf(b + bl, "Size: %llu ", udev->dev_size);
bl += sprintf(b + bl, "MaxDataAreaMB: %u\n",
TCMU_BLOCKS_TO_MBS(udev->max_blocks));
@@ -2323,7 +2318,7 @@ static ssize_t tcmu_dev_size_show(struct config_item *item, char *page)
struct se_dev_attrib, da_group);
struct tcmu_dev *udev = TCMU_DEV(da->da_dev);
- return snprintf(page, PAGE_SIZE, "%zu\n", udev->dev_size);
+ return snprintf(page, PAGE_SIZE, "%llu\n", udev->dev_size);
}
static int tcmu_send_dev_size_event(struct tcmu_dev *udev, u64 size)
--
1.8.3.1
^ permalink raw reply related
* [PATCH 8/9] tcmu: use match_int for dev params
From: Mike Christie @ 2018-07-23 19:07 UTC (permalink / raw)
To: target-devel
Instead of doing strdup and kstrto* just use match_int for dev params.
It will be ok to use int instead of unsigned long in tcmu_set_dev_attrib
because that is only being used for max sectors and block size and the
supported values for them are well under the max possible integer value.
Signed-off-by: Mike Christie <mchristi@redhat.com>
---
drivers/target/target_core_user.c | 37 ++++++++++++++-----------------------
1 file changed, 14 insertions(+), 23 deletions(-)
diff --git a/drivers/target/target_core_user.c b/drivers/target/target_core_user.c
index 969ccba..cfe4f4b 100644
--- a/drivers/target/target_core_user.c
+++ b/drivers/target/target_core_user.c
@@ -2017,8 +2017,8 @@ enum {
static match_table_t tokens = {
{Opt_dev_config, "dev_config=%s"},
{Opt_dev_size, "dev_size=%u"},
- {Opt_hw_block_size, "hw_block_size=%u"},
- {Opt_hw_max_sectors, "hw_max_sectors=%u"},
+ {Opt_hw_block_size, "hw_block_size=%d"},
+ {Opt_hw_max_sectors, "hw_max_sectors=%d"},
{Opt_nl_reply_supported, "nl_reply_supported=%d"},
{Opt_max_data_area_mb, "max_data_area_mb=%d"},
{Opt_err, NULL}
@@ -2026,25 +2026,21 @@ enum {
static int tcmu_set_dev_attrib(substring_t *arg, u32 *dev_attrib)
{
- unsigned long tmp_ul;
- char *arg_p;
- int ret;
-
- arg_p = match_strdup(arg);
- if (!arg_p)
- return -ENOMEM;
+ int val, ret;
- ret = kstrtoul(arg_p, 0, &tmp_ul);
- kfree(arg_p);
+ ret = match_int(arg, &val);
if (ret < 0) {
- pr_err("kstrtoul() failed for dev attrib\n");
+ pr_err("match_int() failed for dev attrib. Error %d.\n",
+ ret);
return ret;
}
- if (!tmp_ul) {
- pr_err("dev attrib must be nonzero\n");
+
+ if (val <= 0) {
+ pr_err("Invalid dev attrib value %d. Must be greater than zero.\n",
+ val);
return -EINVAL;
}
- *dev_attrib = tmp_ul;
+ *dev_attrib = val;
return 0;
}
@@ -2131,15 +2127,10 @@ static ssize_t tcmu_set_configfs_dev_params(struct se_device *dev,
&(dev->dev_attrib.hw_max_sectors));
break;
case Opt_nl_reply_supported:
- arg_p = match_strdup(&args[0]);
- if (!arg_p) {
- ret = -ENOMEM;
- break;
- }
- ret = kstrtoint(arg_p, 0, &udev->nl_reply_supported);
- kfree(arg_p);
+ ret = match_int(&args[0], &udev->nl_reply_supported);
if (ret < 0)
- pr_err("kstrtoint() failed for nl_reply_supported=\n");
+ pr_err("match_int() failed for nl_reply_supported=. Error %d.\n",
+ ret);
break;
case Opt_max_data_area_mb:
ret = tcmu_set_max_blocks_param(udev, &args[0]);
--
1.8.3.1
^ permalink raw reply related
* [PATCH 7/9] tcmu: do not set max_blocks if data_bitmap has been setup
From: Mike Christie @ 2018-07-23 19:07 UTC (permalink / raw)
To: target-devel
This patch prevents a bug where data_bitmap is allocated in
tcmu_configure_device, userspace changes the max_blocks setting, the
device is mapped to a LUN, then we try to access the data_bitmap based
on the new max_blocks limit which may now be out of range.
To prevent this, we just check if data_bitmap has been setup. If it has
then we fail the max_blocks update operation.
Signed-off-by: Mike Christie <mchristi@redhat.com>
---
drivers/target/target_core_user.c | 73 +++++++++++++++++++++------------------
1 file changed, 40 insertions(+), 33 deletions(-)
diff --git a/drivers/target/target_core_user.c b/drivers/target/target_core_user.c
index 31cfe83..969ccba 100644
--- a/drivers/target/target_core_user.c
+++ b/drivers/target/target_core_user.c
@@ -1811,9 +1811,11 @@ static int tcmu_configure_device(struct se_device *dev)
info = &udev->uio_info;
+ mutex_lock(&udev->cmdr_lock);
udev->data_bitmap = kcalloc(BITS_TO_LONGS(udev->max_blocks),
sizeof(unsigned long),
GFP_KERNEL);
+ mutex_unlock(&udev->cmdr_lock);
if (!udev->data_bitmap) {
ret = -ENOMEM;
goto err_bitmap_alloc;
@@ -2018,7 +2020,7 @@ enum {
{Opt_hw_block_size, "hw_block_size=%u"},
{Opt_hw_max_sectors, "hw_max_sectors=%u"},
{Opt_nl_reply_supported, "nl_reply_supported=%d"},
- {Opt_max_data_area_mb, "max_data_area_mb=%u"},
+ {Opt_max_data_area_mb, "max_data_area_mb=%d"},
{Opt_err, NULL}
};
@@ -2046,13 +2048,48 @@ static int tcmu_set_dev_attrib(substring_t *arg, u32 *dev_attrib)
return 0;
}
+static int tcmu_set_max_blocks_param(struct tcmu_dev *udev, substring_t *arg)
+{
+ int val, ret;
+
+ ret = match_int(arg, &val);
+ if (ret < 0) {
+ pr_err("match_int() failed for max_data_area_mb=. Error %d.\n",
+ ret);
+ return ret;
+ }
+
+ if (val <= 0) {
+ pr_err("Invalid max_data_area %d.\n", val);
+ return -EINVAL;
+ }
+
+ mutex_lock(&udev->cmdr_lock);
+ if (udev->data_bitmap) {
+ pr_err("Cannot set max_data_area_mb after it has been enabled.\n");
+ ret = -EINVAL;
+ goto unlock;
+ }
+
+ udev->max_blocks = TCMU_MBS_TO_BLOCKS(val);
+ if (udev->max_blocks > tcmu_global_max_blocks) {
+ pr_err("%d is too large. Adjusting max_data_area_mb to global limit of %u\n",
+ val, TCMU_BLOCKS_TO_MBS(tcmu_global_max_blocks));
+ udev->max_blocks = tcmu_global_max_blocks;
+ }
+
+unlock:
+ mutex_unlock(&udev->cmdr_lock);
+ return ret;
+}
+
static ssize_t tcmu_set_configfs_dev_params(struct se_device *dev,
const char *page, ssize_t count)
{
struct tcmu_dev *udev = TCMU_DEV(dev);
char *orig, *ptr, *opts, *arg_p;
substring_t args[MAX_OPT_ARGS];
- int ret = 0, token, tmpval;
+ int ret = 0, token;
opts = kstrdup(page, GFP_KERNEL);
if (!opts)
@@ -2105,37 +2142,7 @@ static ssize_t tcmu_set_configfs_dev_params(struct se_device *dev,
pr_err("kstrtoint() failed for nl_reply_supported=\n");
break;
case Opt_max_data_area_mb:
- if (dev->export_count) {
- pr_err("Unable to set max_data_area_mb while exports exist\n");
- ret = -EINVAL;
- break;
- }
-
- arg_p = match_strdup(&args[0]);
- if (!arg_p) {
- ret = -ENOMEM;
- break;
- }
- ret = kstrtoint(arg_p, 0, &tmpval);
- kfree(arg_p);
- if (ret < 0) {
- pr_err("kstrtoint() failed for max_data_area_mb=\n");
- break;
- }
-
- if (tmpval <= 0) {
- pr_err("Invalid max_data_area %d\n", tmpval);
- ret = -EINVAL;
- break;
- }
-
- udev->max_blocks = TCMU_MBS_TO_BLOCKS(tmpval);
- if (udev->max_blocks > tcmu_global_max_blocks) {
- pr_err("%d is too large. Adjusting max_data_area_mb to global limit of %u\n",
- tmpval,
- TCMU_BLOCKS_TO_MBS(tcmu_global_max_blocks));
- udev->max_blocks = tcmu_global_max_blocks;
- }
+ ret = tcmu_set_max_blocks_param(udev, &args[0]);
break;
default:
break;
--
1.8.3.1
^ permalink raw reply related
* [PATCH 6/9] tcmu: unmap if dev is configured
From: Mike Christie @ 2018-07-23 19:07 UTC (permalink / raw)
To: target-devel
The tcmu dev is added to the list of tcmu devices during configuration.
At this time the tcmu setup has completed, but lio core has not
completed its setup. The device is not yet usable so do not try to unmap
blocks from it
Signed-off-by: Mike Christie <mchristi@redhat.com>
---
drivers/target/target_core_user.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/target/target_core_user.c b/drivers/target/target_core_user.c
index d6b4022..31cfe83 100644
--- a/drivers/target/target_core_user.c
+++ b/drivers/target/target_core_user.c
@@ -2581,6 +2581,11 @@ static void find_free_blocks(void)
list_for_each_entry(udev, &root_udev, node) {
mutex_lock(&udev->cmdr_lock);
+ if (!target_dev_configured(&udev->se_dev)) {
+ mutex_unlock(&udev->cmdr_lock);
+ continue;
+ }
+
/* Try to complete the finished commands first */
tcmu_handle_completions(udev);
--
1.8.3.1
^ permalink raw reply related
* [PATCH 5/9] tcmu: check if dev is configured before block/reset
From: Mike Christie @ 2018-07-23 19:07 UTC (permalink / raw)
To: target-devel
Do not allow userspace to block or reset the ring until the device has
been configured. This will prevent the bug where userspace can write to
those files and access mb_addr before it has been setup.
Signed-off-by: Mike Christie <mchristi@redhat.com>
---
drivers/target/target_core_user.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/drivers/target/target_core_user.c b/drivers/target/target_core_user.c
index bc8121f..d6b4022 100644
--- a/drivers/target/target_core_user.c
+++ b/drivers/target/target_core_user.c
@@ -2480,6 +2480,11 @@ static ssize_t tcmu_block_dev_store(struct config_item *item, const char *page,
u8 val;
int ret;
+ if (!target_dev_configured(&udev->se_dev)) {
+ pr_err("Device is not configured.\n");
+ return -EINVAL;
+ }
+
ret = kstrtou8(page, 0, &val);
if (ret < 0)
return ret;
@@ -2507,6 +2512,11 @@ static ssize_t tcmu_reset_ring_store(struct config_item *item, const char *page,
u8 val;
int ret;
+ if (!target_dev_configured(&udev->se_dev)) {
+ pr_err("Device is not configured.\n");
+ return -EINVAL;
+ }
+
ret = kstrtou8(page, 0, &val);
if (ret < 0)
return ret;
--
1.8.3.1
^ permalink raw reply related
* [PATCH 4/9] tcmu: use lio core se_device configuration helper
From: Mike Christie @ 2018-07-23 19:07 UTC (permalink / raw)
To: target-devel
Use the lio core helper to check if the device is configured.
Signed-off-by: Mike Christie <mchristi@redhat.com>
---
drivers/target/target_core_user.c | 11 +++--------
1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/drivers/target/target_core_user.c b/drivers/target/target_core_user.c
index b010ed7..bc8121f 100644
--- a/drivers/target/target_core_user.c
+++ b/drivers/target/target_core_user.c
@@ -1908,11 +1908,6 @@ static int tcmu_configure_device(struct se_device *dev)
return ret;
}
-static bool tcmu_dev_configured(struct tcmu_dev *udev)
-{
- return udev->uio_info.uio_dev ? true : false;
-}
-
static void tcmu_free_device(struct se_device *dev)
{
struct tcmu_dev *udev = TCMU_DEV(dev);
@@ -2305,7 +2300,7 @@ static ssize_t tcmu_dev_config_store(struct config_item *item, const char *page,
return -EINVAL;
/* Check if device has been configured before */
- if (tcmu_dev_configured(udev)) {
+ if (target_dev_configured(&udev->se_dev)) {
ret = tcmu_send_dev_config_event(udev, page);
if (ret) {
pr_err("Unable to reconfigure device\n");
@@ -2367,7 +2362,7 @@ static ssize_t tcmu_dev_size_store(struct config_item *item, const char *page,
return ret;
/* Check if device has been configured before */
- if (tcmu_dev_configured(udev)) {
+ if (target_dev_configured(&udev->se_dev)) {
ret = tcmu_send_dev_size_event(udev, val);
if (ret) {
pr_err("Unable to reconfigure device\n");
@@ -2449,7 +2444,7 @@ static ssize_t tcmu_emulate_write_cache_store(struct config_item *item,
return ret;
/* Check if device has been configured before */
- if (tcmu_dev_configured(udev)) {
+ if (target_dev_configured(&udev->se_dev)) {
ret = tcmu_send_emulate_write_cache(udev, val);
if (ret) {
pr_err("Unable to reconfigure device\n");
--
1.8.3.1
^ permalink raw reply related
* [PATCH 3/9] target: add helper to check if dev is configured
From: Mike Christie @ 2018-07-23 19:07 UTC (permalink / raw)
To: target-devel
This just adds a helper function to check if a device is configured and
it converts the target users to use it. The next patch will add a
backend module user so those types of modules do not have to know the
lio core details.
Signed-off-by: Mike Christie <mchristi@redhat.com>
---
drivers/target/target_core_configfs.c | 8 ++++----
drivers/target/target_core_device.c | 6 +++---
drivers/target/target_core_fabric_configfs.c | 3 ++-
include/target/target_core_backend.h | 4 ++++
4 files changed, 13 insertions(+), 8 deletions(-)
diff --git a/drivers/target/target_core_configfs.c b/drivers/target/target_core_configfs.c
index 93d3ff3..f6b1549 100644
--- a/drivers/target/target_core_configfs.c
+++ b/drivers/target/target_core_configfs.c
@@ -810,7 +810,7 @@ static ssize_t pi_prot_type_store(struct config_item *item,
dev->transport->name);
return -ENOSYS;
}
- if (!(dev->dev_flags & DF_CONFIGURED)) {
+ if (!target_dev_configured(dev)) {
pr_err("DIF protection requires device to be configured\n");
return -ENODEV;
}
@@ -859,7 +859,7 @@ static ssize_t pi_prot_format_store(struct config_item *item,
dev->transport->name);
return -ENOSYS;
}
- if (!(dev->dev_flags & DF_CONFIGURED)) {
+ if (!target_dev_configured(dev)) {
pr_err("DIF protection format requires device to be configured\n");
return -ENODEV;
}
@@ -1948,7 +1948,7 @@ static ssize_t target_dev_enable_show(struct config_item *item, char *page)
{
struct se_device *dev = to_device(item);
- return snprintf(page, PAGE_SIZE, "%d\n", !!(dev->dev_flags & DF_CONFIGURED));
+ return snprintf(page, PAGE_SIZE, "%d\n", target_dev_configured(dev));
}
static ssize_t target_dev_enable_store(struct config_item *item,
@@ -2473,7 +2473,7 @@ static ssize_t target_tg_pt_gp_alua_access_state_store(struct config_item *item,
" tg_pt_gp ID: %hu\n", tg_pt_gp->tg_pt_gp_valid_id);
return -EINVAL;
}
- if (!(dev->dev_flags & DF_CONFIGURED)) {
+ if (!target_dev_configured(dev)) {
pr_err("Unable to set alua_access_state while device is"
" not configured\n");
return -ENODEV;
diff --git a/drivers/target/target_core_device.c b/drivers/target/target_core_device.c
index 73675ee..47b5ef1 100644
--- a/drivers/target/target_core_device.c
+++ b/drivers/target/target_core_device.c
@@ -900,7 +900,7 @@ static int target_devices_idr_iter(int id, void *p, void *data)
* to allow other callers to access partially setup devices,
* so we skip them here.
*/
- if (!(dev->dev_flags & DF_CONFIGURED))
+ if (!target_dev_configured(dev))
return 0;
iter->prev_item = config_item_get_unless_zero(&dev->dev_group.cg_item);
@@ -940,7 +940,7 @@ int target_configure_device(struct se_device *dev)
struct se_hba *hba = dev->se_hba;
int ret, id;
- if (dev->dev_flags & DF_CONFIGURED) {
+ if (target_dev_configured(dev)) {
pr_err("se_dev->se_dev_ptr already set for storage"
" object\n");
return -EEXIST;
@@ -1045,7 +1045,7 @@ void target_free_device(struct se_device *dev)
WARN_ON(!list_empty(&dev->dev_sep_list));
- if (dev->dev_flags & DF_CONFIGURED) {
+ if (target_dev_configured(dev)) {
destroy_workqueue(dev->tmr_wq);
dev->transport->destroy_device(dev);
diff --git a/drivers/target/target_core_fabric_configfs.c b/drivers/target/target_core_fabric_configfs.c
index 1fa436e..aa2f4f6 100644
--- a/drivers/target/target_core_fabric_configfs.c
+++ b/drivers/target/target_core_fabric_configfs.c
@@ -34,6 +34,7 @@
#include <linux/configfs.h>
#include <target/target_core_base.h>
+#include <target/target_core_backend.h>
#include <target/target_core_fabric.h>
#include "target_core_internal.h"
@@ -642,7 +643,7 @@ static int target_fabric_port_link(
}
dev = container_of(to_config_group(se_dev_ci), struct se_device, dev_group);
- if (!(dev->dev_flags & DF_CONFIGURED)) {
+ if (!target_dev_configured(dev)) {
pr_err("se_device not configured yet, cannot port link\n");
return -ENODEV;
}
diff --git a/include/target/target_core_backend.h b/include/target/target_core_backend.h
index c3ac472..51b6f50 100644
--- a/include/target/target_core_backend.h
+++ b/include/target/target_core_backend.h
@@ -111,6 +111,10 @@ sense_reason_t passthrough_parse_cdb(struct se_cmd *cmd,
bool target_configure_unmap_from_queue(struct se_dev_attrib *attrib,
struct request_queue *q);
+static inline bool target_dev_configured(struct se_device *se_dev)
+{
+ return !!(se_dev->dev_flags & DF_CONFIGURED);
+}
/* Only use get_unaligned_be24() if reading p - 1 is allowed. */
static inline uint32_t get_unaligned_be24(const uint8_t *const p)
--
1.8.3.1
^ permalink raw reply related
* [PATCH 2/9] tcmu: initialize list head
From: Mike Christie @ 2018-07-23 19:07 UTC (permalink / raw)
To: target-devel
Use INIT_LIST_HEAD to initialize node list head.
Signed-off-by: Mike Christie <mchristi@redhat.com>
---
drivers/target/target_core_user.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/target/target_core_user.c b/drivers/target/target_core_user.c
index fafe65f..b010ed7 100644
--- a/drivers/target/target_core_user.c
+++ b/drivers/target/target_core_user.c
@@ -1342,6 +1342,7 @@ static struct se_device *tcmu_alloc_device(struct se_hba *hba, const char *name)
udev->max_blocks = DATA_BLOCK_BITS_DEF;
mutex_init(&udev->cmdr_lock);
+ INIT_LIST_HEAD(&udev->node);
INIT_LIST_HEAD(&udev->timedout_entry);
INIT_LIST_HEAD(&udev->cmdr_queue);
idr_init(&udev->commands);
--
1.8.3.1
^ permalink raw reply related
* [PATCH 1/9] target_core_user: fix double unlock
From: Mike Christie @ 2018-07-23 19:07 UTC (permalink / raw)
To: target-devel
The caller of queue_cmd_ring grabs and releases the lock, so
the tcmu_setup_cmd_timer failure handling inside queue_cmd_ring
should not call mutex_unlock.
Signed-off-by: Mike Christie <mchristi@redhat.com>
---
drivers/target/target_core_user.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/target/target_core_user.c b/drivers/target/target_core_user.c
index 847707a..fafe65f 100644
--- a/drivers/target/target_core_user.c
+++ b/drivers/target/target_core_user.c
@@ -1066,7 +1066,6 @@ static sense_reason_t queue_cmd_ring(struct tcmu_cmd *tcmu_cmd, int *scsi_err)
&udev->cmd_timer);
if (ret) {
tcmu_cmd_free_data(tcmu_cmd, tcmu_cmd->dbi_cnt);
- mutex_unlock(&udev->cmdr_lock);
*scsi_err = TCM_OUT_OF_RESOURCES;
return -1;
--
1.8.3.1
^ permalink raw reply related
* [PATCH 0/9] tcmu: configuration fixes and cleanups
From: Mike Christie @ 2018-07-23 19:07 UTC (permalink / raw)
To: target-devel
The following patches were made over Martin's for-next branch.
The first patch fixes a locking bug in the command setup path. The rest
of the patches fix several bugs and cleanup the setup and configuration
code paths.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.