Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH v3 2/2] drivers/perf: Add CCPI2 PMU support in ThunderX2 UNCORE driver.
From: Mark Rutland @ 2019-08-13 11:03 UTC (permalink / raw)
  To: Ganapatrao Kulkarni
  Cc: Ganapatrao Kulkarni, corbet@lwn.net, Jan Glauber,
	linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
	Robert Richter, Jayachandran Chandrasekharan Nair,
	will@kernel.org, linux-arm-kernel@lists.infradead.org
In-Reply-To: <CAKTKpr7juHd9Bgam28LESadihFadEAevRAhc-7w3PTMYY7HLNw@mail.gmail.com>

On Tue, Aug 13, 2019 at 04:25:15PM +0530, Ganapatrao Kulkarni wrote:
> Hi Mark,
> 
> On Mon, Aug 12, 2019 at 5:31 PM Mark Rutland <mark.rutland@arm.com> wrote:
> >
> > On Tue, Jul 23, 2019 at 09:16:28AM +0000, Ganapatrao Kulkarni wrote:
> > > CCPI2 is a low-latency high-bandwidth serial interface for connecting
> > > ThunderX2 processors. This patch adds support to capture CCPI2 perf events.
> >
> > It would be worth pointing out in the commit message how the CCPI2
> > counters differ from the others. I realise you have that in the body of
> > patch 1, but it's critical information when reviewing this patch...
> 
> Ok, I will add in next version.
> >
> > >
> > > Signed-off-by: Ganapatrao Kulkarni <gkulkarni@marvell.com>
> > > ---
> > >  drivers/perf/thunderx2_pmu.c | 248 ++++++++++++++++++++++++++++++-----
> > >  1 file changed, 214 insertions(+), 34 deletions(-)
> > >
> > > diff --git a/drivers/perf/thunderx2_pmu.c b/drivers/perf/thunderx2_pmu.c
> > > index 43d76c85da56..a4e1273eafa3 100644
> > > --- a/drivers/perf/thunderx2_pmu.c
> > > +++ b/drivers/perf/thunderx2_pmu.c
> > > @@ -17,22 +17,31 @@
> > >   */
> > >
> > >  #define TX2_PMU_MAX_COUNTERS         4
> >
> > Shouldn't this be 8 now?
> 
> It is kept unchanged to 4(as suggested by Will), which is same for
> both L3 and DMC.
> For CCPI2 this macro is not used.

Hmmm....

I disagree with that suggestion given that this also affects the
active_counters bitmap size (and thus it is not correctly sized as of
this patch), and it doesn't really save us much.

I think it would be better to bump this to 8 and always update the
events array, even though it will be unused for CCPI2. That's less
surprising, needs fewer special-cases, and we can use the hrtimer
function pointer alone to determine if we need to do any hrtimer work.

Thanks,
Mark.

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

^ permalink raw reply

* Re: [PATCH 14/22] ARM: omap1: use pci_ioremap_io() for omap_cf
From: Arnd Bergmann @ 2019-08-13 11:02 UTC (permalink / raw)
  To: Tony Lindgren
  Cc: Aaro Koskinen, Greg Kroah-Hartman, Linus Walleij,
	Bartlomiej Zolnierkiewicz, Linux Kernel Mailing List,
	Dominik Brodowski, Tomi Valkeinen, linux-omap, Linux ARM
In-Reply-To: <20190813103605.GL52127@atomide.com>

On Tue, Aug 13, 2019 at 12:36 PM Tony Lindgren <tony@atomide.com> wrote:

> * Arnd Bergmann <arnd@arndb.de> [190808 21:34]:
> > The ISA I/O space handling in omap_cf is incompatible with
> > PCI drivers in a multiplatform kernel, and requires a custom
> > mach/io.h.
> >
> > Change the driver to use pci_ioremap_io() like PCI drivers do,
> > so the generic ioport access can work across platforms.
> >
> > To actually use that code, we have to select CONFIG_PCI
> > here.
> >
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>
> Looks like this series boots for me on 5912osk up to this
> patch

Ok, that's something. Thanks for testing!

>, but this patch breaks booting somehow.
>
> Any ideas for fixes?

I can think of multiple possible issues:

- I force CONFIG_PCI to be enabled here in order to keep the
  asm/io.h logic unchanged. If PCI support in itself is an issue,
  then turning on CONFIG_PCI without the rest of this patch
  should also break.

- I see that osk is the one board that actually has a CF slot.
  If it crashes while trying to access a CF device, then I most
  likely screwed up the I/O port mapping itself, so it uses the
  wrong virtual and/or physical address for the access.

- I made on the assumption that only CF devices use inb/outb
  on any OMAP1 machine. If there is a driver that uses inb/outb
  instead of readb/writeb, it would have worked by chance before
  but stopped working now.

        Arnd

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

^ permalink raw reply

* RE: [EXT] Re: [PATCHv5 1/2] PCI: layerscape: Add the bar_fixed_64bit property in EP driver.
From: Xiaowei Bao @ 2019-08-13 11:02 UTC (permalink / raw)
  To: Lorenzo Pieralisi
  Cc: linux-arm-kernel@lists.infradead.org, Roy Zang, Leonard Crestez,
	hayashi.kunihiko@socionext.com, andrew.smirnov@gmail.com,
	linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org,
	yue.wang@amlogic.com, kishon@ti.com, M.h. Lian, dwmw@amazon.co.uk,
	jonnyc@amazon.com, bhelgaas@google.com, tpiepho@impinj.com,
	linuxppc-dev@lists.ozlabs.org, Mingkai Hu, l.stach@pengutronix.de
In-Reply-To: <20190813100409.GB10070@red-moon>



> -----Original Message-----
> From: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Sent: 2019年8月13日 18:04
> To: Xiaowei Bao <xiaowei.bao@nxp.com>
> Cc: bhelgaas@google.com; M.h. Lian <minghuan.lian@nxp.com>; Mingkai Hu
> <mingkai.hu@nxp.com>; Roy Zang <roy.zang@nxp.com>;
> l.stach@pengutronix.de; kishon@ti.com; tpiepho@impinj.com; Leonard
> Crestez <leonard.crestez@nxp.com>; andrew.smirnov@gmail.com;
> yue.wang@amlogic.com; hayashi.kunihiko@socionext.com;
> dwmw@amazon.co.uk; jonnyc@amazon.com; linux-pci@vger.kernel.org;
> linux-kernel@vger.kernel.org; linuxppc-dev@lists.ozlabs.org;
> linux-arm-kernel@lists.infradead.org
> Subject: [EXT] Re: [PATCHv5 1/2] PCI: layerscape: Add the bar_fixed_64bit
> property in EP driver.
> 
> Caution: EXT Email
> 
> git log --oneline --follow drivers/pci/controller/dwc/pci-layerscape.c
> 
> Do you see any commit with a $SUBJECT ending with a period ?
> 
> There is not. So remove it from yours too.
OK, thanks a lot, I will remove it in the next version patch, I have to get the approved
Form IT team of our company. 
> 
> On Tue, Aug 13, 2019 at 02:28:39PM +0800, Xiaowei Bao wrote:
> > The PCIe controller of layerscape just have 4 BARs, BAR0 and BAR1 is
> > 32bit, BAR2 and BAR4 is 64bit, this is determined by hardware, so set
> > the bar_fixed_64bit with 0x14.
> >
> > Signed-off-by: Xiaowei Bao <xiaowei.bao@nxp.com>
> > ---
> > v2:
> >  - Replace value 0x14 with a macro.
> > v3:
> >  - No change.
> > v4:
> >  - send the patch again with '--to'.
> > v5:
> >  - fix the commit message.
> >
> >  drivers/pci/controller/dwc/pci-layerscape-ep.c | 1 +
> >  1 file changed, 1 insertion(+)
> 
> scripts/get_maintainer.pl -f drivers/pci/controller/dwc/pci-layerscape-ep.c
> Now, with the output you get justify all the people you send this email to.
> 
> So, again, trim the CC list and it is the last time I tell you.
Do you mean that I use scripts/get_maintainer.pl -f drivers/pci/controller/
dwc/pci-layerscape-ep.c to get the mail list who I need to send? I use the
command of ' scripts/get_maintainer.pl *.patch' to get the mail list before.
If yes, I will use the command that you provided. Thanks a lot.
> 
> Before sending patches on mailing lists use git --dry-run to check the emails
> you are sending.
> 
> Thanks,
> Lorenzo
> 
> > diff --git a/drivers/pci/controller/dwc/pci-layerscape-ep.c
> > b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> > index be61d96..ca9aa45 100644
> > --- a/drivers/pci/controller/dwc/pci-layerscape-ep.c
> > +++ b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> > @@ -44,6 +44,7 @@ static const struct pci_epc_features
> ls_pcie_epc_features = {
> >       .linkup_notifier = false,
> >       .msi_capable = true,
> >       .msix_capable = false,
> > +     .bar_fixed_64bit = (1 << BAR_2) | (1 << BAR_4),
> >  };
> >
> >  static const struct pci_epc_features*
> > --
> > 2.9.5
> >
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v2 2/3] media: i2c: Add IMX290 CMOS image sensor driver
From: Sakari Ailus @ 2019-08-13 10:59 UTC (permalink / raw)
  To: Manivannan Sadhasivam
  Cc: devicetree, c.barrett, linux-kernel, a.brela, robh+dt, mchehab,
	linux-arm-kernel, linux-media
In-Reply-To: <20190806130938.19916-3-manivannan.sadhasivam@linaro.org>

Hi Manivannan,

On Tue, Aug 06, 2019 at 06:39:37PM +0530, Manivannan Sadhasivam wrote:
> Add driver for Sony IMX290 CMOS image sensor driver. The driver only
> supports I2C interface for programming and MIPI CSI-2 for sensor output.
> 
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> ---
>  drivers/media/i2c/Kconfig  |  11 +
>  drivers/media/i2c/Makefile |   1 +
>  drivers/media/i2c/imx290.c | 845 +++++++++++++++++++++++++++++++++++++
>  3 files changed, 857 insertions(+)
>  create mode 100644 drivers/media/i2c/imx290.c
> 
> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
> index cb8db944aa41..256edd289abe 100644
> --- a/drivers/media/i2c/Kconfig
> +++ b/drivers/media/i2c/Kconfig
> @@ -594,6 +594,17 @@ config VIDEO_IMX274
>  	  This is a V4L2 sensor driver for the Sony IMX274
>  	  CMOS image sensor.
>  
> +config VIDEO_IMX290
> +	tristate "Sony IMX290 sensor support"
> +	depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API
> +	depends on MEDIA_CAMERA_SUPPORT
> +	help
> +	  This is a Video4Linux2 sensor driver for the Sony
> +	  IMX290 camera sensor.
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called imx290.
> +
>  config VIDEO_IMX319
>  	tristate "Sony IMX319 sensor support"
>  	depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API
> diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
> index d8ad9dad495d..490e59070407 100644
> --- a/drivers/media/i2c/Makefile
> +++ b/drivers/media/i2c/Makefile
> @@ -111,6 +111,7 @@ obj-$(CONFIG_VIDEO_TC358743)	+= tc358743.o
>  obj-$(CONFIG_VIDEO_IMX214)	+= imx214.o
>  obj-$(CONFIG_VIDEO_IMX258)	+= imx258.o
>  obj-$(CONFIG_VIDEO_IMX274)	+= imx274.o
> +obj-$(CONFIG_VIDEO_IMX290)	+= imx290.o
>  obj-$(CONFIG_VIDEO_IMX319)	+= imx319.o
>  obj-$(CONFIG_VIDEO_IMX355)	+= imx355.o
>  obj-$(CONFIG_VIDEO_ST_MIPID02) += st-mipid02.o
> diff --git a/drivers/media/i2c/imx290.c b/drivers/media/i2c/imx290.c
> new file mode 100644
> index 000000000000..8c50fbd51ca5
> --- /dev/null
> +++ b/drivers/media/i2c/imx290.c
> @@ -0,0 +1,845 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Sony IMX290 CMOS Image Sensor Driver
> + *
> + * Copyright (C) 2019 FRAMOS GmbH.
> + *
> + * Copyright (C) 2019 Linaro Ltd.
> + * Author: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/i2c.h>
> +#include <linux/module.h>
> +#include <linux/regmap.h>
> +#include <linux/regulator/consumer.h>
> +#include <media/media-entity.h>
> +#include <media/v4l2-ctrls.h>
> +#include <media/v4l2-device.h>
> +#include <media/v4l2-fwnode.h>
> +#include <media/v4l2-subdev.h>
> +
> +#define IMX290_STANDBY 0x3000
> +#define IMX290_REGHOLD 0x3001
> +#define IMX290_XMSTA 0x3002
> +#define IMX290_GAIN 0x3014
> +
> +static const char * const imx290_supply_name[] = {
> +	"vdda",
> +	"vddd",
> +	"vdddo",
> +};
> +
> +#define IMX290_NUM_SUPPLIES ARRAY_SIZE(imx290_supply_name)
> +
> +struct imx290_regval {
> +	u16 reg;
> +	u8 val;
> +};
> +
> +struct imx290_mode {
> +	u32 width;
> +	u32 height;
> +	u32 pixel_rate;
> +	u32 link_freq_index;
> +
> +	const struct imx290_regval *data;
> +	u32 data_size;
> +};
> +
> +struct imx290 {
> +	struct device *dev;
> +	struct clk *xclk;
> +	struct regmap *regmap;
> +
> +	struct v4l2_subdev sd;
> +	struct v4l2_fwnode_endpoint ep;
> +	struct media_pad pad;
> +	struct v4l2_mbus_framefmt current_format;
> +	const struct imx290_mode *current_mode;
> +
> +	struct regulator_bulk_data supplies[IMX290_NUM_SUPPLIES];
> +	struct gpio_desc *rst_gpio;
> +
> +	struct v4l2_ctrl_handler ctrls;
> +	struct v4l2_ctrl *link_freq;
> +	struct v4l2_ctrl *pixel_rate;
> +
> +	struct mutex lock;
> +};
> +
> +struct imx290_pixfmt {
> +	u32 code;
> +	u32 colorspace;
> +};
> +
> +static const struct imx290_pixfmt imx290_formats[] = {
> +	{ MEDIA_BUS_FMT_SRGGB10_1X10, V4L2_COLORSPACE_SRGB, },

I bet you won't use other colorspaces in this driver. Therefore there's
there's no need to put it in an array.

> +};
> +
> +static struct regmap_config imx290_regmap_config = {
> +	.reg_bits = 16,
> +	.val_bits = 8,
> +	.cache_type = REGCACHE_RBTREE,
> +};
> +
> +static const struct imx290_regval imx290_global_init_settings[] = {
> +	{ 0x3007, 0x00 },
> +	{ 0x3009, 0x00 },
> +	{ 0x3018, 0x65 },
> +	{ 0x3019, 0x04 },
> +	{ 0x301a, 0x00 },
> +	{ 0x3443, 0x03 },
> +	{ 0x3444, 0x20 },
> +	{ 0x3445, 0x25 },
> +	{ 0x3407, 0x03 },
> +	{ 0x303a, 0x0c },
> +	{ 0x3040, 0x00 },
> +	{ 0x3041, 0x00 },
> +	{ 0x303c, 0x00 },
> +	{ 0x303d, 0x00 },
> +	{ 0x3042, 0x9c },
> +	{ 0x3043, 0x07 },
> +	{ 0x303e, 0x49 },
> +	{ 0x303f, 0x04 },
> +	{ 0x304b, 0x0a },
> +	{ 0x300f, 0x00 },
> +	{ 0x3010, 0x21 },
> +	{ 0x3012, 0x64 },
> +	{ 0x3016, 0x09 },
> +	{ 0x3070, 0x02 },
> +	{ 0x3071, 0x11 },
> +	{ 0x309b, 0x10 },
> +	{ 0x309c, 0x22 },
> +	{ 0x30a2, 0x02 },
> +	{ 0x30a6, 0x20 },
> +	{ 0x30a8, 0x20 },
> +	{ 0x30aa, 0x20 },
> +	{ 0x30ac, 0x20 },
> +	{ 0x30b0, 0x43 },
> +	{ 0x3119, 0x9e },
> +	{ 0x311c, 0x1e },
> +	{ 0x311e, 0x08 },
> +	{ 0x3128, 0x05 },
> +	{ 0x313d, 0x83 },
> +	{ 0x3150, 0x03 },
> +	{ 0x317e, 0x00 },
> +	{ 0x32b8, 0x50 },
> +	{ 0x32b9, 0x10 },
> +	{ 0x32ba, 0x00 },
> +	{ 0x32bb, 0x04 },
> +	{ 0x32c8, 0x50 },
> +	{ 0x32c9, 0x10 },
> +	{ 0x32ca, 0x00 },
> +	{ 0x32cb, 0x04 },
> +	{ 0x332c, 0xd3 },
> +	{ 0x332d, 0x10 },
> +	{ 0x332e, 0x0d },
> +	{ 0x3358, 0x06 },
> +	{ 0x3359, 0xe1 },
> +	{ 0x335a, 0x11 },
> +	{ 0x3360, 0x1e },
> +	{ 0x3361, 0x61 },
> +	{ 0x3362, 0x10 },
> +	{ 0x33b0, 0x50 },
> +	{ 0x33b2, 0x1a },
> +	{ 0x33b3, 0x04 },
> +};
> +
> +static const struct imx290_regval imx290_1080p_settings[] = {
> +	/* mode settings */
> +	{ 0x3007, 0x00 },
> +	{ 0x303a, 0x0c },
> +	{ 0x3414, 0x0a },
> +	{ 0x3472, 0x80 },
> +	{ 0x3473, 0x07 },
> +	{ 0x3418, 0x38 },
> +	{ 0x3419, 0x04 },
> +	{ 0x3012, 0x64 },
> +	{ 0x3013, 0x00 },
> +	{ 0x305c, 0x18 },
> +	{ 0x305d, 0x03 },
> +	{ 0x305e, 0x20 },
> +	{ 0x305f, 0x01 },
> +	{ 0x315e, 0x1a },
> +	{ 0x3164, 0x1a },
> +	{ 0x3480, 0x49 },
> +	/* data rate settings */
> +	{ 0x3009, 0x01 },
> +	{ 0x3405, 0x10 },
> +	{ 0x3446, 0x57 },
> +	{ 0x3447, 0x00 },
> +	{ 0x3448, 0x37 },
> +	{ 0x3449, 0x00 },
> +	{ 0x344a, 0x1f },
> +	{ 0x344b, 0x00 },
> +	{ 0x344c, 0x1f },
> +	{ 0x344d, 0x00 },
> +	{ 0x344e, 0x1f },
> +	{ 0x344f, 0x00 },
> +	{ 0x3450, 0x77 },
> +	{ 0x3451, 0x00 },
> +	{ 0x3452, 0x1f },
> +	{ 0x3453, 0x00 },
> +	{ 0x3454, 0x17 },
> +	{ 0x3455, 0x00 },
> +	{ 0x301c, 0x98 },
> +	{ 0x301d, 0x08 },
> +};
> +
> +static const struct imx290_regval imx290_720p_settings[] = {
> +	/* mode settings */
> +	{ 0x3007, 0x10 },
> +	{ 0x303a, 0x06 },
> +	{ 0x3414, 0x04 },
> +	{ 0x3472, 0x00 },
> +	{ 0x3473, 0x05 },
> +	{ 0x3418, 0xd0 },
> +	{ 0x3419, 0x02 },
> +	{ 0x3012, 0x64 },
> +	{ 0x3013, 0x00 },
> +	{ 0x305c, 0x20 },
> +	{ 0x305d, 0x00 },
> +	{ 0x305e, 0x20 },
> +	{ 0x305f, 0x01 },
> +	{ 0x315e, 0x1a },
> +	{ 0x3164, 0x1a },
> +	{ 0x3480, 0x49 },
> +	/* data rate settings */
> +	{ 0x3009, 0x01 },
> +	{ 0x3405, 0x10 },
> +	{ 0x3446, 0x4f },
> +	{ 0x3447, 0x00 },
> +	{ 0x3448, 0x2f },
> +	{ 0x3449, 0x00 },
> +	{ 0x344a, 0x17 },
> +	{ 0x344b, 0x00 },
> +	{ 0x344c, 0x17 },
> +	{ 0x344d, 0x00 },
> +	{ 0x344e, 0x17 },
> +	{ 0x344f, 0x00 },
> +	{ 0x3450, 0x57 },
> +	{ 0x3451, 0x00 },
> +	{ 0x3452, 0x17 },
> +	{ 0x3453, 0x00 },
> +	{ 0x3454, 0x17 },
> +	{ 0x3455, 0x00 },
> +	{ 0x301c, 0xe4 },
> +	{ 0x301d, 0x0c },
> +};
> +
> +static const struct imx290_regval imx290_10bit_settings[] = {
> +	{ 0x3005, 0x00},
> +	{ 0x3046, 0x00},
> +	{ 0x3129, 0x1d},
> +	{ 0x317c, 0x12},
> +	{ 0x31ec, 0x37},
> +	{ 0x3441, 0x0a},
> +	{ 0x3442, 0x0a},
> +	{ 0x300a, 0x3c},
> +	{ 0x300b, 0x00},
> +};
> +
> +/* supported link frequencies */
> +static const s64 imx290_link_freq[] = {
> +	445500000,
> +};
> +
> +/* Mode configs */
> +static const struct imx290_mode imx290_modes[] = {
> +	{
> +		.width = 1920,
> +		.height = 1080,
> +		.data = imx290_1080p_settings,
> +		.data_size = ARRAY_SIZE(imx290_1080p_settings),
> +		.pixel_rate = 178200000,
> +		.link_freq_index = 0,
> +	},
> +	{
> +		.width = 1280,
> +		.height = 720,
> +		.data = imx290_720p_settings,
> +		.data_size = ARRAY_SIZE(imx290_720p_settings),
> +		.pixel_rate = 178200000,
> +		.link_freq_index = 0,
> +	},
> +};
> +
> +static inline struct imx290 *to_imx290(struct v4l2_subdev *_sd)
> +{
> +	return container_of(_sd, struct imx290, sd);
> +}
> +
> +static inline int imx290_read_reg(struct imx290 *imx290, u16 addr, u8 *value)
> +{
> +	u32 regval;
> +	int ret;
> +
> +	ret = regmap_read(imx290->regmap, addr, &regval);
> +	if (ret) {
> +		dev_err(imx290->dev, "I2C read failed for addr: %x\n", addr);
> +		return ret;
> +	}
> +
> +	*value = regval & 0xFF;

Lower case hexadecimals are preferred.

> +
> +	return 0;
> +}
> +
> +static int imx290_write_reg(struct imx290 *imx290, u16 addr, u8 value)
> +{
> +	int ret;
> +
> +	ret = regmap_write(imx290->regmap, addr, value);
> +	if (ret) {
> +		dev_err(imx290->dev, "I2C write failed for addr: %x\n", addr);
> +		return ret;
> +	}
> +
> +	return ret;
> +}
> +
> +static int imx290_set_register_array(struct imx290 *imx290,
> +				     const struct imx290_regval *settings,
> +				     unsigned int num_settings)
> +{
> +	unsigned int i;
> +	int ret;
> +
> +	for (i = 0; i < num_settings; ++i, ++settings) {
> +		ret = imx290_write_reg(imx290, settings->reg, settings->val);
> +		if (ret < 0)
> +			return ret;
> +
> +		/* Settle time is 10ms for all registers */
> +		msleep(10);
> +	}
> +
> +	return 0;
> +}
> +
> +static int imx290_write_buffered_reg(struct imx290 *imx290, u16 address_low,
> +				     u8 nr_regs, u32 value)
> +{
> +	int ret, i;

unsigned int i

> +	u8 val = 0;
> +
> +	ret = imx290_write_reg(imx290, IMX290_REGHOLD, 0x01);
> +	if (ret) {
> +		dev_err(imx290->dev, "Error setting hold register\n");
> +		return ret;
> +	}
> +
> +	for (i = 0; i < nr_regs; i++) {
> +		imx290_read_reg(imx290, address_low + i, &val);

What's the purpose of reading back val, and doing the same again below?

You could do this in a much more simple way in a single write, see e.g.
ov5670_write_reg().

> +		ret = imx290_write_reg(imx290, address_low + i,
> +				       (u8)(value >> (i * 8)));
> +		if (ret) {
> +			dev_err(imx290->dev, "Error writing buffered registers\n");
> +			return ret;
> +		}
> +		imx290_read_reg(imx290, address_low + i, &val);
> +	}
> +
> +	ret = imx290_write_reg(imx290, IMX290_REGHOLD, 0x00);
> +	if (ret) {
> +		dev_err(imx290->dev, "Error setting hold register\n");
> +		return ret;
> +	}
> +
> +	return ret;
> +}
> +
> +static int imx290_set_gain(struct imx290 *imx290, u32 value)
> +{
> +	int ret;
> +
> +	u32 adjusted_value = (value * 10) / 3;
> +
> +	ret = imx290_write_buffered_reg(imx290, IMX290_GAIN, 1, adjusted_value);
> +	if (ret)
> +		dev_err(imx290->dev, "Unable to write gain\n");
> +
> +	return ret;
> +}
> +
> +static int imx290_set_power_on(struct imx290 *imx290)
> +{
> +	int ret;
> +
> +	ret = clk_prepare_enable(imx290->xclk);
> +	if (ret) {
> +		dev_err(imx290->dev, "Failed to enable clock\n");
> +		return ret;
> +	}
> +
> +	ret = regulator_bulk_enable(IMX290_NUM_SUPPLIES,
> +				    imx290->supplies);
> +	if (ret) {
> +		dev_err(imx290->dev, "Failed to enable regulators\n");
> +		goto xclk_off;
> +	}
> +
> +	usleep_range(1, 2);
> +	gpiod_set_value_cansleep(imx290->rst_gpio, 1);
> +	usleep_range(30000, 31000);
> +
> +	return 0;
> +
> +xclk_off:
> +	clk_disable_unprepare(imx290->xclk);
> +	return ret;
> +}
> +
> +/* Stop streaming */
> +static int imx290_stop_streaming(struct imx290 *imx290)
> +{
> +	int ret;
> +
> +	ret = imx290_write_reg(imx290, IMX290_STANDBY, 0x01);
> +	if (ret < 0)
> +		return ret;
> +
> +	msleep(30);
> +
> +	return imx290_write_reg(imx290, IMX290_XMSTA, 0x01);
> +}
> +
> +static void imx290_set_power_off(struct imx290 *imx290)
> +{
> +	clk_disable_unprepare(imx290->xclk);
> +	gpiod_set_value_cansleep(imx290->rst_gpio, 0);
> +	regulator_bulk_disable(IMX290_NUM_SUPPLIES, imx290->supplies);
> +}
> +
> +static int imx290_s_power(struct v4l2_subdev *sd, int on)
> +{
> +	struct imx290 *imx290 = to_imx290(sd);
> +	int ret = 0;
> +
> +	mutex_lock(&imx290->lock);
> +
> +	if (on) {
> +		ret = imx290_set_power_on(imx290);
> +		if (ret < 0)
> +			goto exit;
> +
> +		ret = imx290_set_register_array(imx290,
> +				imx290_global_init_settings,
> +				ARRAY_SIZE(imx290_global_init_settings));
> +		if (ret < 0) {
> +			dev_err(imx290->dev,
> +				"Could not set init registers\n");
> +			imx290_set_power_off(imx290);
> +			goto exit;
> +		}
> +
> +		imx290_stop_streaming(imx290);
> +	} else {
> +		imx290_set_power_off(imx290);
> +	}
> +
> +exit:
> +	mutex_unlock(&imx290->lock);
> +
> +	return ret;

Please use runtime PM instead. It simplifies the driver. See e.g.
drivers/media/i2c/ov5670.c or drivers/media/i2c/ov5695.c .

> +}
> +
> +static int imx290_set_ctrl(struct v4l2_ctrl *ctrl)
> +{
> +	struct imx290 *imx290 = container_of(ctrl->handler,
> +					     struct imx290, ctrls);
> +	int ret = 0;
> +
> +	mutex_lock(&imx290->lock);
> +
> +	switch (ctrl->id) {
> +	case V4L2_CID_GAIN:
> +		ret = imx290_set_gain(imx290, ctrl->val);
> +		break;
> +	default:
> +		dev_info(imx290->dev, "ctrl(id:0x%x,val:0x%x) is not handled",
> +			 ctrl->id, ctrl->val);

Please return an error; I'd do it instead of printing that.

> +		break;
> +	}
> +
> +	mutex_unlock(&imx290->lock);
> +
> +	return ret;
> +}
> +
> +static const struct v4l2_ctrl_ops imx290_ctrl_ops = {
> +	.s_ctrl = imx290_set_ctrl,
> +};
> +
> +static int imx290_enum_mbus_code(struct v4l2_subdev *sd,
> +				 struct v4l2_subdev_pad_config *cfg,
> +				 struct v4l2_subdev_mbus_code_enum *code)
> +{
> +	if (code->index >= ARRAY_SIZE(imx290_formats))
> +		return -EINVAL;
> +
> +	code->code = imx290_formats[code->index].code;
> +
> +	return 0;
> +}
> +
> +static int imx290_get_fmt(struct v4l2_subdev *sd,
> +			  struct v4l2_subdev_pad_config *cfg,
> +			  struct v4l2_subdev_format *fmt)
> +{
> +	struct imx290 *imx290 = to_imx290(sd);
> +	struct v4l2_mbus_framefmt *framefmt;
> +
> +	if (fmt->which == V4L2_SUBDEV_FORMAT_TRY)
> +		framefmt = v4l2_subdev_get_try_format(&imx290->sd, cfg,
> +						      fmt->pad);
> +	else
> +		framefmt = &imx290->current_format;
> +
> +	fmt->format = *framefmt;
> +
> +	return 0;
> +}
> +
> +static int imx290_set_fmt(struct v4l2_subdev *sd,
> +			  struct v4l2_subdev_pad_config *cfg,
> +		      struct v4l2_subdev_format *fmt)
> +{
> +	struct imx290 *imx290 = to_imx290(sd);
> +	const struct imx290_mode *mode;
> +	struct v4l2_mbus_framefmt *format;
> +	int i, ret = 0;

Note that sub-device drivers need to serialise access through the uAPI to
their own data.

> +
> +	mode = v4l2_find_nearest_size(imx290_modes,
> +				      ARRAY_SIZE(imx290_modes),
> +				      width, height,
> +				      fmt->format.width, fmt->format.height);
> +
> +	fmt->format.width = mode->width;
> +	fmt->format.height = mode->height;
> +
> +	for (i = 0; i < ARRAY_SIZE(imx290_formats); i++)
> +		if (imx290_formats[i].code == fmt->format.code)
> +			break;
> +
> +	if (i >= ARRAY_SIZE(imx290_formats))
> +		i = 0;
> +
> +	fmt->format.code = imx290_formats[i].code;
> +	fmt->format.colorspace = imx290_formats[i].colorspace;
> +	fmt->format.field = V4L2_FIELD_NONE;
> +
> +	if (fmt->which == V4L2_SUBDEV_FORMAT_TRY) {
> +		format = v4l2_subdev_get_try_format(sd, cfg, fmt->pad);
> +	} else {
> +		format = &imx290->current_format;
> +		__v4l2_ctrl_s_ctrl(imx290->link_freq, mode->link_freq_index);
> +		__v4l2_ctrl_s_ctrl_int64(imx290->pixel_rate, mode->pixel_rate);
> +
> +		imx290->current_mode = mode;
> +	}
> +
> +	*format = fmt->format;
> +
> +	return ret;
> +}
> +
> +static int imx290_entity_init_cfg(struct v4l2_subdev *subdev,
> +				  struct v4l2_subdev_pad_config *cfg)
> +{
> +	struct v4l2_subdev_format fmt = { 0 };
> +
> +	fmt.which = cfg ? V4L2_SUBDEV_FORMAT_TRY : V4L2_SUBDEV_FORMAT_ACTIVE;
> +	fmt.format.width = 1920;
> +	fmt.format.height = 1080;
> +
> +	imx290_set_fmt(subdev, cfg, &fmt);
> +
> +	return 0;
> +}
> +
> +static int imx290_write_current_format(struct imx290 *imx290,
> +				       struct v4l2_mbus_framefmt *format)
> +{
> +	int ret;
> +
> +	switch (format->code) {
> +	case MEDIA_BUS_FMT_SRGGB10_1X10:
> +		ret = imx290_set_register_array(imx290, imx290_10bit_settings,
> +					ARRAY_SIZE(imx290_10bit_settings));
> +		if (ret < 0) {
> +			dev_err(imx290->dev, "Could not set format registers\n");
> +			return ret;
> +		}
> +		break;
> +	default:
> +		dev_err(imx290->dev, "Unknown pixel format\n");
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +/* Start streaming */
> +static int imx290_start_streaming(struct imx290 *imx290)
> +{
> +	int ret;
> +
> +	/* Set current frame format */
> +	ret = imx290_write_current_format(imx290, &imx290->current_format);
> +	if (ret < 0) {
> +		dev_err(imx290->dev, "Could not set frame format\n");
> +		return ret;
> +	}
> +
> +	/* Apply default values of current mode */
> +	ret = imx290_set_register_array(imx290, imx290->current_mode->data,
> +					imx290->current_mode->data_size);
> +	if (ret < 0) {
> +		dev_err(imx290->dev, "Could not set current mode\n");
> +		return ret;
> +	}
> +
> +	/* Apply customized values from user */
> +	ret = v4l2_ctrl_handler_setup(imx290->sd.ctrl_handler);
> +	if (ret) {
> +		dev_err(imx290->dev, "Could not sync v4l2 controls\n");
> +		return ret;
> +	}
> +
> +	ret = imx290_write_reg(imx290, IMX290_STANDBY, 0x00);
> +	if (ret < 0)
> +		return ret;
> +
> +	msleep(30);
> +
> +	/* Start streaming */
> +	return imx290_write_reg(imx290, IMX290_XMSTA, 0x00);
> +}
> +
> +static int imx290_set_stream(struct v4l2_subdev *sd, int enable)
> +{
> +	struct imx290 *imx290 = to_imx290(sd);
> +	int ret = 0;
> +
> +	if (enable)
> +		ret = imx290_start_streaming(imx290);
> +	else
> +		imx290_stop_streaming(imx290);
> +
> +	return ret;
> +}
> +
> +static int imx290_get_regulators(struct device *dev, struct imx290 *imx290)
> +{
> +	unsigned int i;
> +
> +	for (i = 0; i < IMX290_NUM_SUPPLIES; i++)
> +		imx290->supplies[i].supply = imx290_supply_name[i];
> +
> +	return devm_regulator_bulk_get(dev, IMX290_NUM_SUPPLIES,
> +				       imx290->supplies);
> +}
> +
> +static const struct v4l2_subdev_core_ops imx290_subdev_core_ops = {
> +	.s_power = imx290_s_power,
> +};
> +
> +static const struct v4l2_subdev_video_ops imx290_video_ops = {
> +	.s_stream = imx290_set_stream,
> +};
> +
> +static const struct v4l2_subdev_pad_ops imx290_pad_ops = {
> +	.init_cfg = imx290_entity_init_cfg,
> +	.enum_mbus_code = imx290_enum_mbus_code,
> +	.get_fmt = imx290_get_fmt,
> +	.set_fmt = imx290_set_fmt,
> +};
> +
> +static const struct v4l2_subdev_ops imx290_subdev_ops = {
> +	.core = &imx290_subdev_core_ops,
> +	.video = &imx290_video_ops,
> +	.pad = &imx290_pad_ops,
> +};
> +
> +static const struct media_entity_operations imx290_subdev_entity_ops = {
> +	.link_validate = v4l2_subdev_link_validate,
> +};
> +
> +static int imx290_probe(struct i2c_client *client,
> +			const struct i2c_device_id *id)
> +{
> +	struct device *dev = &client->dev;
> +	struct fwnode_handle *endpoint;
> +	struct imx290 *imx290;
> +	u32 xclk_freq;
> +	int ret;
> +
> +	imx290 = devm_kzalloc(dev, sizeof(*imx290), GFP_KERNEL);
> +	if (!imx290)
> +		return -ENOMEM;
> +
> +	imx290->dev = dev;
> +	imx290->regmap = devm_regmap_init_i2c(client, &imx290_regmap_config);
> +	if (IS_ERR(imx290->regmap)) {
> +		dev_err(dev, "Unable to initialize I2C\n");
> +		return -ENODEV;
> +	}
> +
> +	endpoint = fwnode_graph_get_next_endpoint(dev_fwnode(dev), NULL);
> +	if (!endpoint) {
> +		dev_err(dev, "Endpoint node not found\n");
> +		return -EINVAL;
> +	}
> +
> +	ret = v4l2_fwnode_endpoint_parse(endpoint, &imx290->ep);
> +	fwnode_handle_put(endpoint);
> +	if (ret < 0) {
> +		dev_err(dev, "Parsing endpoint node failed\n");
> +		return ret;
> +	}
> +
> +	/* Only CSI2 is supported for now */
> +	if (imx290->ep.bus_type != V4L2_MBUS_CSI2_DPHY) {
> +		dev_err(dev, "Unsupported bus type, should be CSI2\n");
> +		return -EINVAL;
> +	}
> +
> +	/* Set default mode to max resolution */
> +	imx290->current_mode = &imx290_modes[0];
> +
> +	/* get system clock (xclk) */
> +	imx290->xclk = devm_clk_get(dev, "xclk");
> +	if (IS_ERR(imx290->xclk)) {
> +		dev_err(dev, "Could not get xclk");
> +		return PTR_ERR(imx290->xclk);
> +	}
> +
> +	ret = of_property_read_u32(dev->of_node, "clock-frequency", &xclk_freq);
> +	if (ret) {
> +		dev_err(dev, "Could not get xclk frequency\n");
> +		return ret;
> +	}
> +
> +	/* external clock must be 37.125 MHz */
> +	if (xclk_freq != 37125000) {
> +		dev_err(dev, "External clock frequency %u is not supported\n",
> +			xclk_freq);
> +		return -EINVAL;
> +	}
> +
> +	ret = clk_set_rate(imx290->xclk, xclk_freq);
> +	if (ret) {
> +		dev_err(dev, "Could not set xclk frequency\n");
> +		return ret;
> +	}
> +
> +	ret = imx290_get_regulators(dev, imx290);
> +	if (ret < 0) {
> +		dev_err(dev, "Cannot get regulators\n");
> +		return ret;
> +	}
> +
> +	imx290->rst_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_ASIS);
> +	if (IS_ERR(imx290->rst_gpio)) {
> +		dev_err(dev, "Cannot get reset gpio\n");

Remember to put the regulators from now on. Or grab them later.

> +		return PTR_ERR(imx290->rst_gpio);
> +	}
> +
> +	mutex_init(&imx290->lock);
> +
> +	v4l2_ctrl_handler_init(&imx290->ctrls, 3);
> +
> +	v4l2_ctrl_new_std(&imx290->ctrls, &imx290_ctrl_ops,
> +			  V4L2_CID_GAIN, 0, 72, 1, 0);
> +	imx290->link_freq = v4l2_ctrl_new_int_menu(&imx290->ctrls,
> +					&imx290_ctrl_ops,
> +					V4L2_CID_LINK_FREQ,
> +					ARRAY_SIZE(imx290_link_freq) - 1,
> +					0, imx290_link_freq);
> +	if (imx290->link_freq)
> +		imx290->link_freq->flags |= V4L2_CTRL_FLAG_READ_ONLY;
> +
> +	imx290->pixel_rate = v4l2_ctrl_new_std(&imx290->ctrls, &imx290_ctrl_ops,
> +					       V4L2_CID_PIXEL_RATE, 1,
> +					       INT_MAX, 1,
> +					       imx290_modes[0].pixel_rate);
> +
> +	imx290->sd.ctrl_handler = &imx290->ctrls;
> +
> +	if (imx290->ctrls.error) {
> +		dev_err(dev, "Control initialization error %d\n",
> +			imx290->ctrls.error);
> +		ret = imx290->ctrls.error;
> +		goto free_ctrl;
> +	}
> +
> +	v4l2_i2c_subdev_init(&imx290->sd, client, &imx290_subdev_ops);
> +	imx290->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
> +	imx290->sd.dev = &client->dev;
> +	imx290->sd.entity.ops = &imx290_subdev_entity_ops;
> +	imx290->sd.entity.function = MEDIA_ENT_F_CAM_SENSOR;
> +
> +	imx290->pad.flags = MEDIA_PAD_FL_SOURCE;
> +	ret = media_entity_pads_init(&imx290->sd.entity, 1, &imx290->pad);
> +	if (ret < 0) {
> +		dev_err(dev, "Could not register media entity\n");
> +		goto free_ctrl;
> +	}
> +
> +	ret = v4l2_async_register_subdev(&imx290->sd);
> +	if (ret < 0) {
> +		dev_err(dev, "Could not register v4l2 device\n");
> +		goto free_entity;
> +	}
> +
> +	return 0;
> +
> +free_entity:
> +	media_entity_cleanup(&imx290->sd.entity);
> +free_ctrl:
> +	v4l2_ctrl_handler_free(&imx290->ctrls);
> +	mutex_destroy(&imx290->lock);
> +
> +	return ret;
> +}
> +
> +static int imx290_remove(struct i2c_client *client)
> +{
> +	struct v4l2_subdev *sd = i2c_get_clientdata(client);
> +	struct imx290 *imx290 = to_imx290(sd);
> +
> +	v4l2_async_unregister_subdev(sd);
> +	media_entity_cleanup(&sd->entity);
> +	v4l2_ctrl_handler_free(sd->ctrl_handler);
> +
> +	imx290_set_power_off(imx290);
> +	mutex_destroy(&imx290->lock);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id imx290_of_match[] = {
> +	{ .compatible = "sony,imx290" },
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, imx290_of_match);
> +
> +static struct i2c_driver imx290_i2c_driver = {
> +	.probe  = imx290_probe,

Please use probe_new instead.

> +	.remove = imx290_remove,
> +	.driver = {
> +		.name  = "imx290",
> +		.of_match_table = of_match_ptr(imx290_of_match),
> +	},
> +};
> +
> +module_i2c_driver(imx290_i2c_driver);
> +
> +MODULE_DESCRIPTION("Sony IMX290 CMOS Image Sensor Driver");
> +MODULE_AUTHOR("FRAMOS GmbH");
> +MODULE_AUTHOR("Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>");
> +MODULE_LICENSE("GPL v2");

-- 
Kind regards,

Sakari Ailus

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

^ permalink raw reply

* Re: "arm64/for-next/core" causes boot panic
From: Will Deacon @ 2019-08-13 10:58 UTC (permalink / raw)
  To: Qian Cai
  Cc: Catalin Marinas, linux-kernel, linux-arm-kernel, Andrey Konovalov
In-Reply-To: <20190813090200.h2rz4xphgnb5j5bc@willie-the-truck>

On Tue, Aug 13, 2019 at 10:02:01AM +0100, Will Deacon wrote:
> On Mon, Aug 12, 2019 at 05:51:35PM -0400, Qian Cai wrote:
> > Booting today's linux-next on an arm64 server triggers a panic with
> > CONFIG_KASAN_SW_TAGS=y pointing to this line,
> 
> Is this the only change on top of defconfig? If not, please can you share
> your full .config?
> 
> > kfree()->virt_to_head_page()->compound_head()
> > 
> > unsigned long head = READ_ONCE(page->compound_head);
> > 
> > The bisect so far indicates one of those could be bad,
> 
> I guess that means the issue is reproducible on the arm64 for-next/core
> branch. Once I have your .config, I'll give it a go.

FWIW, I've managed to reproduce this using defconfig + SW_TAGS on
for-next/core, so I'll keep investigating.

Will

--->8

[    0.000000] Unable to handle kernel paging request at virtual address 0037fe0007580d08
[    0.000000] Mem abort info:
[    0.000000]   ESR = 0x96000004
[    0.000000]   EC = 0x25: DABT (current EL), IL = 32 bits
[    0.000000]   SET = 0, FnV = 0
[    0.000000]   EA = 0, S1PTW = 0
[    0.000000] Data abort info:
[    0.000000]   ISV = 0, ISS = 0x00000004
[    0.000000]   CM = 0, WnR = 0
[    0.000000] [0037fe0007580d08] address between user and kernel address ranges
[    0.000000] Internal error: Oops: 96000004 [#1] PREEMPT SMP
[    0.000000] Modules linked in:
[    0.000000] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.3.0-rc3-00049-gf964cbd07098 #1
[    0.000000] Hardware name: linux,dummy-virt (DT)
[    0.000000] pstate: 20000085 (nzCv daIf -PAN -UAO)
[    0.000000] pc : kfree+0x44/0x6ac
[    0.000000] lr : apply_wqattrs_prepare+0x390/0x3fc
[    0.000000] sp : ffff9000541d7d00
[    0.000000] x29: ffff9000541d7d80 x28: 4dff0001de034e08 
[    0.000000] x27: b2ff0001de040000 x26: 0000000000000004 
[    0.000000] x25: c1ff0001de034c28 x24: 4dff0001de034e00 
[    0.000000] x23: a8ff0001de034d00 x22: c1ff0001de020a00 
[    0.000000] x21: a8ff0001de034d08 x20: 0000000000000000 
[    0.000000] x19: c1ff0001de034c00 x18: 0000000000000000 
[    0.000000] x17: 0000000000000000 x16: 0000000000000000 
[    0.000000] x15: 1ffff6b000000000 x14: ffff900053ca87e4 
[    0.000000] x13: ffff900052539444 x12: ffff90005253ce48 
[    0.000000] x11: 00000000000000c1 x10: ffff80001de034c1 
[    0.000000] x9 : fffffdffffe00008 x8 : 0138000007780d00 
[    0.000000] x7 : ffffffffffffffff x6 : a8ff0001de034d28 
[    0.000000] x5 : 0000000000000040 x4 : 0000000000000008 
[    0.000000] x3 : 0000000000000100 x2 : ffff9000541ddf68 
[    0.000000] x1 : a8ff0001de034d08 x0 : 4dff0001de034e00 
[    0.000000] Call trace:
[    0.000000]  kfree+0x44/0x6ac
[    0.000000]  apply_wqattrs_prepare+0x390/0x3fc
[    0.000000]  apply_workqueue_attrs+0x70/0xe4
[    0.000000]  alloc_workqueue+0x514/0x728
[    0.000000]  workqueue_init_early+0x36c/0x4a0
[    0.000000]  start_kernel+0x1d0/0x46c
[    0.000000] Code: f2bffc09 d346fd08 f2dfbfe9 927acd08 (f8696909) 
[    0.000000] random: get_random_bytes called from oops_exit+0x4c/0x78 with crng_init=0
[    0.000000] ---[ end trace 0000000000000000 ]---


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

^ permalink raw reply

* Re: [PATCH V15 13/13] PCI: tegra: Add Tegra194 PCIe support
From: Lorenzo Pieralisi @ 2019-08-13 10:57 UTC (permalink / raw)
  To: Vidya Sagar
  Cc: mark.rutland, devicetree, mperttunen, mmaddireddy, linux-pci,
	catalin.marinas, will.deacon, linux-kernel, kthota, kishon,
	linux-tegra, robh+dt, thierry.reding, gustavo.pimentel,
	jingoohan1, bhelgaas, digetx, jonathanh, linux-arm-kernel,
	sagar.tv
In-Reply-To: <20190809044609.20401-14-vidyas@nvidia.com>

Some style comments - we have time to fix them.

On Fri, Aug 09, 2019 at 10:16:09AM +0530, Vidya Sagar wrote:

[...]

I do not know why up to here the line spacing is OK and here
you started cramming code all together :)

Just a matter of consistency, thanks for fixing them up.

> +static void tegra_pcie_downstream_dev_to_D0(struct tegra_pcie_dw *pcie)
> +{
> +	struct pcie_port *pp = &pcie->pci.pp;
> +	struct pci_bus *child;
> +	struct pci_dev *pdev;
> +
> +	/*
> +	 * link doesn't go into L2 state with some of the endpoints with Tegra
> +	 * if they are not in D0 state. So, need to make sure that immediate
> +	 * downstream devices are in D0 state before sending PME_TurnOff to put
> +	 * link into L2 state.
> +	 * This is as per PCI Express Base r4.0 v1.0 September 27-2017,
> +	 * 5.2 Link State Power Management (Page #428).
> +	 */
> +
> +	list_for_each_entry(child, &pp->root_bus->children, node) {
> +		/* Bring downstream devices to D0 if they are not already in */
> +		if (child->parent == pp->root_bus)
> +			break;
> +	}

Add empty line.

> +	list_for_each_entry(pdev, &child->devices, bus_list) {
> +		if (PCI_SLOT(pdev->devfn) == 0) {
> +			if (pci_set_power_state(pdev, PCI_D0))
> +				dev_err(pcie->dev,
> +					"Failed to transition %s to D0 state\n",
> +					dev_name(&pdev->dev));
> +		}
> +	}
> +}
> +
> +static int tegra_pcie_config_controller(struct tegra_pcie_dw *pcie,
> +					bool en_hw_hot_rst)
> +{
> +	int ret;
> +	u32 val;
> +
> +	ret = tegra_pcie_bpmp_set_ctrl_state(pcie, true);
> +	if (ret) {
> +		dev_err(pcie->dev,
> +			"Failed to enable controller %u: %d\n", pcie->cid, ret);
> +		return ret;
> +	}
> +
> +	ret = regulator_enable(pcie->pex_ctl_supply);
> +	if (ret < 0) {
> +		dev_err(pcie->dev, "Failed to enable regulator: %d\n", ret);
> +		goto fail_reg_en;
> +	}
> +
> +	ret = clk_prepare_enable(pcie->core_clk);
> +	if (ret) {
> +		dev_err(pcie->dev, "Failed to enable core clock: %d\n", ret);
> +		goto fail_core_clk;
> +	}
> +
> +	ret = reset_control_deassert(pcie->core_apb_rst);
> +	if (ret) {
> +		dev_err(pcie->dev, "Failed to deassert core APB reset: %d\n",
> +			ret);
> +		goto fail_core_apb_rst;
> +	}
> +
> +	if (en_hw_hot_rst) {
> +		/* Enable HW_HOT_RST mode */
> +		val = appl_readl(pcie, APPL_CTRL);
> +		val &= ~(APPL_CTRL_HW_HOT_RST_MODE_MASK <<
> +			 APPL_CTRL_HW_HOT_RST_MODE_SHIFT);
> +		val |= APPL_CTRL_HW_HOT_RST_EN;
> +		appl_writel(pcie, val, APPL_CTRL);
> +	}
> +
> +	ret = tegra_pcie_enable_phy(pcie);
> +	if (ret) {
> +		dev_err(pcie->dev, "Failed to enable PHY: %d\n", ret);
> +		goto fail_phy;
> +	}
> +
> +	/* Update CFG base address */
> +	appl_writel(pcie, pcie->dbi_res->start & APPL_CFG_BASE_ADDR_MASK,
> +		    APPL_CFG_BASE_ADDR);
> +
> +	/* Configure this core for RP mode operation */
> +	appl_writel(pcie, APPL_DM_TYPE_RP, APPL_DM_TYPE);
> +
> +	appl_writel(pcie, 0x0, APPL_CFG_SLCG_OVERRIDE);
> +
> +	val = appl_readl(pcie, APPL_CTRL);
> +	appl_writel(pcie, val | APPL_CTRL_SYS_PRE_DET_STATE, APPL_CTRL);
> +
> +	val = appl_readl(pcie, APPL_CFG_MISC);
> +	val |= (APPL_CFG_MISC_ARCACHE_VAL << APPL_CFG_MISC_ARCACHE_SHIFT);
> +	appl_writel(pcie, val, APPL_CFG_MISC);
> +
> +	if (!pcie->supports_clkreq) {
> +		val = appl_readl(pcie, APPL_PINMUX);
> +		val |= APPL_PINMUX_CLKREQ_OUT_OVRD_EN;
> +		val |= APPL_PINMUX_CLKREQ_OUT_OVRD;
> +		appl_writel(pcie, val, APPL_PINMUX);
> +	}
> +
> +	/* Update iATU_DMA base address */
> +	appl_writel(pcie,
> +		    pcie->atu_dma_res->start & APPL_CFG_IATU_DMA_BASE_ADDR_MASK,
> +		    APPL_CFG_IATU_DMA_BASE_ADDR);
> +
> +	reset_control_deassert(pcie->core_rst);
> +
> +	pcie->pcie_cap_base = dw_pcie_find_capability(&pcie->pci,
> +						      PCI_CAP_ID_EXP);
> +
> +	/* Disable ASPM-L1SS advertisement as there is no CLKREQ routing */
> +	if (!pcie->supports_clkreq) {
> +		disable_aspm_l11(pcie);
> +		disable_aspm_l12(pcie);
> +	}
> +
> +	return ret;
> +
> +fail_phy:
> +	reset_control_assert(pcie->core_apb_rst);
> +fail_core_apb_rst:
> +	clk_disable_unprepare(pcie->core_clk);
> +fail_core_clk:
> +	regulator_disable(pcie->pex_ctl_supply);
> +fail_reg_en:
> +	tegra_pcie_bpmp_set_ctrl_state(pcie, false);
> +
> +	return ret;
> +}
> +
> +static int __deinit_controller(struct tegra_pcie_dw *pcie)
> +{
> +	int ret;
> +
> +	ret = reset_control_assert(pcie->core_rst);
> +	if (ret) {
> +		dev_err(pcie->dev, "Failed to assert \"core\" reset: %d\n",
> +			ret);
> +		return ret;
> +	}

Add empty line.

> +	tegra_pcie_disable_phy(pcie);

Add empty line.

> +	ret = reset_control_assert(pcie->core_apb_rst);
> +	if (ret) {
> +		dev_err(pcie->dev, "Failed to assert APB reset: %d\n", ret);
> +		return ret;
> +	}

Add empty line.

> +	clk_disable_unprepare(pcie->core_clk);

Add empty line.

> +	ret = regulator_disable(pcie->pex_ctl_supply);
> +	if (ret) {
> +		dev_err(pcie->dev, "Failed to disable regulator: %d\n", ret);
> +		return ret;
> +	}

Add empty line.

> +	ret = tegra_pcie_bpmp_set_ctrl_state(pcie, false);
> +	if (ret) {
> +		dev_err(pcie->dev, "Failed to disable controller %d: %d\n",
> +			pcie->cid, ret);
> +		return ret;
> +	}

Add empty line.

> +	return ret;
> +}
> +
> +static int tegra_pcie_init_controller(struct tegra_pcie_dw *pcie)
> +{
> +	struct dw_pcie *pci = &pcie->pci;
> +	struct pcie_port *pp = &pci->pp;
> +	int ret;
> +
> +	ret = tegra_pcie_config_controller(pcie, false);
> +	if (ret < 0)
> +		return ret;
> +
> +	pp->ops = &tegra_pcie_dw_host_ops;
> +
> +	ret = dw_pcie_host_init(pp);
> +	if (ret < 0) {
> +		dev_err(pcie->dev, "Failed to add PCIe port: %d\n", ret);
> +		goto fail_host_init;
> +	}
> +
> +	return 0;
> +
> +fail_host_init:
> +	return __deinit_controller(pcie);
> +}
> +
> +static int tegra_pcie_try_link_l2(struct tegra_pcie_dw *pcie)
> +{
> +	u32 val;
> +
> +	if (!tegra_pcie_dw_link_up(&pcie->pci))
> +		return 0;
> +
> +	val = appl_readl(pcie, APPL_RADM_STATUS);
> +	val |= APPL_PM_XMT_TURNOFF_STATE;
> +	appl_writel(pcie, val, APPL_RADM_STATUS);
> +
> +	return readl_poll_timeout_atomic(pcie->appl_base + APPL_DEBUG, val,
> +				 val & APPL_DEBUG_PM_LINKST_IN_L2_LAT,
> +				 1, PME_ACK_TIMEOUT);
> +}
> +
> +static void tegra_pcie_dw_pme_turnoff(struct tegra_pcie_dw *pcie)
> +{
> +	u32 data;
> +	int err;
> +
> +	if (!tegra_pcie_dw_link_up(&pcie->pci)) {
> +		dev_dbg(pcie->dev, "PCIe link is not up...!\n");
> +		return;
> +	}
> +
> +	if (tegra_pcie_try_link_l2(pcie)) {
> +		dev_info(pcie->dev, "Link didn't transition to L2 state\n");
> +		/*
> +		 * TX lane clock freq will reset to Gen1 only if link is in L2
> +		 * or detect state.
> +		 * So apply pex_rst to end point to force RP to go into detect
> +		 * state
> +		 */
> +		data = appl_readl(pcie, APPL_PINMUX);
> +		data &= ~APPL_PINMUX_PEX_RST;
> +		appl_writel(pcie, data, APPL_PINMUX);
> +
> +		err = readl_poll_timeout_atomic(pcie->appl_base + APPL_DEBUG,
> +						data,
> +						((data &
> +						APPL_DEBUG_LTSSM_STATE_MASK) >>
> +						APPL_DEBUG_LTSSM_STATE_SHIFT) ==
> +						LTSSM_STATE_PRE_DETECT,
> +						1, LTSSM_TIMEOUT);
> +		if (err) {
> +			dev_info(pcie->dev, "Link didn't go to detect state\n");
> +		} else {
> +			/* Disable LTSSM after link is in detect state */
> +			data = appl_readl(pcie, APPL_CTRL);
> +			data &= ~APPL_CTRL_LTSSM_EN;
> +			appl_writel(pcie, data, APPL_CTRL);
> +		}
> +	}
> +	/*
> +	 * DBI registers may not be accessible after this as PLL-E would be
> +	 * down depending on how CLKREQ is pulled by end point
> +	 */
> +	data = appl_readl(pcie, APPL_PINMUX);
> +	data |= (APPL_PINMUX_CLKREQ_OVERRIDE_EN | APPL_PINMUX_CLKREQ_OVERRIDE);
> +	/* Cut REFCLK to slot */
> +	data |= APPL_PINMUX_CLK_OUTPUT_IN_OVERRIDE_EN;
> +	data &= ~APPL_PINMUX_CLK_OUTPUT_IN_OVERRIDE;
> +	appl_writel(pcie, data, APPL_PINMUX);
> +}
> +
> +static int tegra_pcie_deinit_controller(struct tegra_pcie_dw *pcie)
> +{
> +	tegra_pcie_downstream_dev_to_D0(pcie);
> +	dw_pcie_host_deinit(&pcie->pci.pp);
> +	tegra_pcie_dw_pme_turnoff(pcie);

Add empty line.

> +	return __deinit_controller(pcie);
> +}
> +
> +static int tegra_pcie_config_rp(struct tegra_pcie_dw *pcie)
> +{
> +	struct pcie_port *pp = &pcie->pci.pp;
> +	struct device *dev = pcie->dev;
> +	char *name;
> +	int ret;
> +
> +	if (IS_ENABLED(CONFIG_PCI_MSI)) {
> +		pp->msi_irq = of_irq_get_byname(dev->of_node, "msi");
> +		if (!pp->msi_irq) {
> +			dev_err(dev, "Failed to get MSI interrupt\n");
> +			return -ENODEV;
> +		}
> +	}
> +
> +	pm_runtime_enable(dev);
> +	ret = pm_runtime_get_sync(dev);
> +	if (ret < 0) {
> +		dev_err(dev, "Failed to get runtime sync for PCIe dev: %d\n",
> +			ret);
> +		pm_runtime_disable(dev);
> +		return ret;
> +	}
> +
> +	tegra_pcie_init_controller(pcie);
> +
> +	pcie->link_state = tegra_pcie_dw_link_up(&pcie->pci);
> +
> +	if (!pcie->link_state) {
> +		ret = -ENOMEDIUM;
> +		goto fail_host_init;
> +	}
> +
> +	name = devm_kasprintf(dev, GFP_KERNEL, "%pOFP", dev->of_node);
> +	if (!name) {
> +		ret = -ENOMEM;
> +		goto fail_host_init;
> +	}
> +
> +	pcie->debugfs = debugfs_create_dir(name, NULL);
> +	if (!pcie->debugfs)
> +		dev_err(dev, "Failed to create debugfs\n");
> +	else
> +		init_debugfs(pcie);
> +
> +	return ret;
> +
> +fail_host_init:
> +	tegra_pcie_deinit_controller(pcie);
> +	pm_runtime_put_sync(dev);
> +	pm_runtime_disable(dev);
> +	return ret;
> +}
> +
> +static int tegra_pcie_dw_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct resource *atu_dma_res;
> +	struct tegra_pcie_dw *pcie;
> +	struct resource *dbi_res;
> +	struct pcie_port *pp;
> +	struct dw_pcie *pci;
> +	struct phy **phys;
> +	char *name;
> +	int ret;
> +	u32 i;
> +
> +	pcie = devm_kzalloc(dev, sizeof(*pcie), GFP_KERNEL);
> +	if (!pcie)
> +		return -ENOMEM;
> +
> +	pci = &pcie->pci;
> +	pci->dev = &pdev->dev;
> +	pci->ops = &tegra_dw_pcie_ops;
> +	pp = &pci->pp;
> +	pcie->dev = &pdev->dev;
> +
> +	ret = tegra_pcie_dw_parse_dt(pcie);
> +	if (ret < 0) {
> +		dev_err(dev, "Failed to parse device tree: %d\n", ret);
> +		return ret;
> +	}
> +
> +	pcie->pex_ctl_supply = devm_regulator_get(dev, "vddio-pex-ctl");
> +	if (IS_ERR(pcie->pex_ctl_supply)) {
> +		dev_err(dev, "Failed to get regulator: %ld\n",
> +			PTR_ERR(pcie->pex_ctl_supply));
> +		return PTR_ERR(pcie->pex_ctl_supply);
> +	}
> +
> +	pcie->core_clk = devm_clk_get(dev, "core");
> +	if (IS_ERR(pcie->core_clk)) {
> +		dev_err(dev, "Failed to get core clock: %ld\n",
> +			PTR_ERR(pcie->core_clk));
> +		return PTR_ERR(pcie->core_clk);
> +	}
> +
> +	pcie->appl_res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> +						      "appl");
> +	if (!pcie->appl_res) {
> +		dev_err(dev, "Failed to find \"appl\" region\n");
> +		return PTR_ERR(pcie->appl_res);
> +	}
> +
> +	pcie->appl_base = devm_ioremap_resource(dev, pcie->appl_res);
> +	if (IS_ERR(pcie->appl_base))
> +		return PTR_ERR(pcie->appl_base);
> +
> +	pcie->core_apb_rst = devm_reset_control_get(dev, "apb");
> +	if (IS_ERR(pcie->core_apb_rst)) {
> +		dev_err(dev, "Failed to get APB reset: %ld\n",
> +			PTR_ERR(pcie->core_apb_rst));
> +		return PTR_ERR(pcie->core_apb_rst);
> +	}
> +
> +	phys = devm_kcalloc(dev, pcie->phy_count, sizeof(*phys), GFP_KERNEL);
> +	if (!phys)
> +		return PTR_ERR(phys);
> +
> +	for (i = 0; i < pcie->phy_count; i++) {
> +		name = kasprintf(GFP_KERNEL, "p2u-%u", i);
> +		if (!name) {
> +			dev_err(dev, "Failed to create P2U string\n");
> +			return -ENOMEM;
> +		}
> +		phys[i] = devm_phy_get(dev, name);
> +		kfree(name);
> +		if (IS_ERR(phys[i])) {
> +			ret = PTR_ERR(phys[i]);
> +			dev_err(dev, "Failed to get PHY: %d\n", ret);
> +			return ret;
> +		}
> +	}
> +
> +	pcie->phys = phys;
> +
> +	dbi_res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dbi");
> +	if (!dbi_res) {
> +		dev_err(dev, "Failed to find \"dbi\" region\n");
> +		return PTR_ERR(dbi_res);
> +	}
> +	pcie->dbi_res = dbi_res;
> +
> +	pci->dbi_base = devm_ioremap_resource(dev, dbi_res);
> +	if (IS_ERR(pci->dbi_base))
> +		return PTR_ERR(pci->dbi_base);
> +
> +	/* Tegra HW locates DBI2 at a fixed offset from DBI */
> +	pci->dbi_base2 = pci->dbi_base + 0x1000;
> +
> +	atu_dma_res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> +						   "atu_dma");
> +	if (!atu_dma_res) {
> +		dev_err(dev, "Failed to find \"atu_dma\" region\n");
> +		return PTR_ERR(atu_dma_res);
> +	}
> +	pcie->atu_dma_res = atu_dma_res;
> +
> +	pci->atu_base = devm_ioremap_resource(dev, atu_dma_res);
> +	if (IS_ERR(pci->atu_base))
> +		return PTR_ERR(pci->atu_base);
> +
> +	pcie->core_rst = devm_reset_control_get(dev, "core");
> +	if (IS_ERR(pcie->core_rst)) {
> +		dev_err(dev, "Failed to get core reset: %ld\n",
> +			PTR_ERR(pcie->core_rst));
> +		return PTR_ERR(pcie->core_rst);
> +	}
> +
> +	pp->irq = platform_get_irq_byname(pdev, "intr");
> +	if (!pp->irq) {
> +		dev_err(dev, "Failed to get \"intr\" interrupt\n");
> +		return -ENODEV;
> +	}
> +
> +	ret = devm_request_irq(dev, pp->irq, tegra_pcie_irq_handler,
> +			       IRQF_SHARED, "tegra-pcie-intr", pcie);
> +	if (ret) {
> +		dev_err(dev, "Failed to request IRQ %d: %d\n", pp->irq, ret);
> +		return ret;
> +	}
> +
> +	pcie->bpmp = tegra_bpmp_get(dev);
> +	if (IS_ERR(pcie->bpmp))
> +		return PTR_ERR(pcie->bpmp);
> +
> +	platform_set_drvdata(pdev, pcie);
> +
> +	ret = tegra_pcie_config_rp(pcie);
> +	if (ret && ret != -ENOMEDIUM)
> +		goto fail;
> +	else
> +		return 0;
> +
> +fail:
> +	tegra_bpmp_put(pcie->bpmp);
> +	return ret;
> +}
> +
> +static int tegra_pcie_dw_remove(struct platform_device *pdev)
> +{
> +	struct tegra_pcie_dw *pcie = platform_get_drvdata(pdev);
> +
> +	if (!pcie->link_state)
> +		return 0;
> +
> +	debugfs_remove_recursive(pcie->debugfs);
> +	tegra_pcie_deinit_controller(pcie);
> +	pm_runtime_put_sync(pcie->dev);
> +	pm_runtime_disable(pcie->dev);
> +	tegra_bpmp_put(pcie->bpmp);
> +
> +	return 0;
> +}
> +
> +static int tegra_pcie_dw_suspend_late(struct device *dev)
> +{
> +	struct tegra_pcie_dw *pcie = dev_get_drvdata(dev);
> +	u32 val;
> +
> +	if (!pcie->link_state)
> +		return 0;
> +
> +	/* Enable HW_HOT_RST mode */
> +	val = appl_readl(pcie, APPL_CTRL);
> +	val &= ~(APPL_CTRL_HW_HOT_RST_MODE_MASK <<
> +		 APPL_CTRL_HW_HOT_RST_MODE_SHIFT);
> +	val |= APPL_CTRL_HW_HOT_RST_EN;
> +	appl_writel(pcie, val, APPL_CTRL);
> +
> +	return 0;
> +}
> +
> +static int tegra_pcie_dw_suspend_noirq(struct device *dev)
> +{
> +	struct tegra_pcie_dw *pcie = dev_get_drvdata(dev);
> +
> +	if (!pcie->link_state)
> +		return 0;
> +
> +	/* Save MSI interrupt vector */
> +	pcie->msi_ctrl_int = dw_pcie_readl_dbi(&pcie->pci,
> +					       PORT_LOGIC_MSI_CTRL_INT_0_EN);
> +	tegra_pcie_downstream_dev_to_D0(pcie);
> +	tegra_pcie_dw_pme_turnoff(pcie);

Add empty line.

> +	return __deinit_controller(pcie);
> +}
> +
> +static int tegra_pcie_dw_resume_noirq(struct device *dev)
> +{
> +	struct tegra_pcie_dw *pcie = dev_get_drvdata(dev);
> +	int ret;
> +
> +	if (!pcie->link_state)
> +		return 0;
> +
> +	ret = tegra_pcie_config_controller(pcie, true);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = tegra_pcie_dw_host_init(&pcie->pci.pp);
> +	if (ret < 0) {
> +		dev_err(dev, "Failed to init host: %d\n", ret);
> +		goto fail_host_init;
> +	}
> +
> +	/* Restore MSI interrupt vector */
> +	dw_pcie_writel_dbi(&pcie->pci, PORT_LOGIC_MSI_CTRL_INT_0_EN,
> +			   pcie->msi_ctrl_int);
> +
> +	return 0;

Add empty line.

Thanks,
Lorenzo

> +fail_host_init:
> +	return __deinit_controller(pcie);
> +}
> +
> +static int tegra_pcie_dw_resume_early(struct device *dev)
> +{
> +	struct tegra_pcie_dw *pcie = dev_get_drvdata(dev);
> +	u32 val;
> +
> +	if (!pcie->link_state)
> +		return 0;
> +
> +	/* Disable HW_HOT_RST mode */
> +	val = appl_readl(pcie, APPL_CTRL);
> +	val &= ~(APPL_CTRL_HW_HOT_RST_MODE_MASK <<
> +		 APPL_CTRL_HW_HOT_RST_MODE_SHIFT);
> +	val |= APPL_CTRL_HW_HOT_RST_MODE_IMDT_RST <<
> +	       APPL_CTRL_HW_HOT_RST_MODE_SHIFT;
> +	val &= ~APPL_CTRL_HW_HOT_RST_EN;
> +	appl_writel(pcie, val, APPL_CTRL);
> +
> +	return 0;
> +}
> +
> +static void tegra_pcie_dw_shutdown(struct platform_device *pdev)
> +{
> +	struct tegra_pcie_dw *pcie = platform_get_drvdata(pdev);
> +
> +	if (!pcie->link_state)
> +		return;
> +
> +	debugfs_remove_recursive(pcie->debugfs);
> +	tegra_pcie_downstream_dev_to_D0(pcie);
> +
> +	disable_irq(pcie->pci.pp.irq);
> +	if (IS_ENABLED(CONFIG_PCI_MSI))
> +		disable_irq(pcie->pci.pp.msi_irq);
> +
> +	tegra_pcie_dw_pme_turnoff(pcie);
> +	__deinit_controller(pcie);
> +}
> +
> +static const struct of_device_id tegra_pcie_dw_of_match[] = {
> +	{
> +		.compatible = "nvidia,tegra194-pcie",
> +	},
> +	{},
> +};
> +
> +static const struct dev_pm_ops tegra_pcie_dw_pm_ops = {
> +	.suspend_late = tegra_pcie_dw_suspend_late,
> +	.suspend_noirq = tegra_pcie_dw_suspend_noirq,
> +	.resume_noirq = tegra_pcie_dw_resume_noirq,
> +	.resume_early = tegra_pcie_dw_resume_early,
> +};
> +
> +static struct platform_driver tegra_pcie_dw_driver = {
> +	.probe = tegra_pcie_dw_probe,
> +	.remove = tegra_pcie_dw_remove,
> +	.shutdown = tegra_pcie_dw_shutdown,
> +	.driver = {
> +		.name	= "tegra194-pcie",
> +		.pm = &tegra_pcie_dw_pm_ops,
> +		.of_match_table = tegra_pcie_dw_of_match,
> +	},
> +};
> +module_platform_driver(tegra_pcie_dw_driver);
> +
> +MODULE_DEVICE_TABLE(of, tegra_pcie_dw_of_match);
> +
> +MODULE_AUTHOR("Vidya Sagar <vidyas@nvidia.com>");
> +MODULE_DESCRIPTION("NVIDIA PCIe host controller driver");
> +MODULE_LICENSE("GPL v2");
> -- 
> 2.17.1
> 

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

^ permalink raw reply

* Re: [PATCH v3 2/2] drivers/perf: Add CCPI2 PMU support in ThunderX2 UNCORE driver.
From: Ganapatrao Kulkarni @ 2019-08-13 10:55 UTC (permalink / raw)
  To: Mark Rutland
  Cc: Ganapatrao Kulkarni, corbet@lwn.net, Jan Glauber,
	linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
	Robert Richter, Jayachandran Chandrasekharan Nair,
	will@kernel.org, linux-arm-kernel@lists.infradead.org
In-Reply-To: <20190812120125.GA50712@lakrids.cambridge.arm.com>

Hi Mark,

On Mon, Aug 12, 2019 at 5:31 PM Mark Rutland <mark.rutland@arm.com> wrote:
>
> On Tue, Jul 23, 2019 at 09:16:28AM +0000, Ganapatrao Kulkarni wrote:
> > CCPI2 is a low-latency high-bandwidth serial interface for connecting
> > ThunderX2 processors. This patch adds support to capture CCPI2 perf events.
>
> It would be worth pointing out in the commit message how the CCPI2
> counters differ from the others. I realise you have that in the body of
> patch 1, but it's critical information when reviewing this patch...

Ok, I will add in next version.
>
> >
> > Signed-off-by: Ganapatrao Kulkarni <gkulkarni@marvell.com>
> > ---
> >  drivers/perf/thunderx2_pmu.c | 248 ++++++++++++++++++++++++++++++-----
> >  1 file changed, 214 insertions(+), 34 deletions(-)
> >
> > diff --git a/drivers/perf/thunderx2_pmu.c b/drivers/perf/thunderx2_pmu.c
> > index 43d76c85da56..a4e1273eafa3 100644
> > --- a/drivers/perf/thunderx2_pmu.c
> > +++ b/drivers/perf/thunderx2_pmu.c
> > @@ -17,22 +17,31 @@
> >   */
> >
> >  #define TX2_PMU_MAX_COUNTERS         4
>
> Shouldn't this be 8 now?

It is kept unchanged to 4(as suggested by Will), which is same for
both L3 and DMC.
For CCPI2 this macro is not used.

>
> [...]
>
> >  /*
> > - * pmu on each socket has 2 uncore devices(dmc and l3c),
> > - * each device has 4 counters.
> > + * pmu on each socket has 3 uncore devices(dmc, l3ci and ccpi2),
> > + * dmc and l3c has 4 counters and ccpi2 8.
> >   */
>
> How about:
>
> /*
>  * Each socket has 3 uncore device associated with a PMU. The DMC and
>  * L3C have 4 32-bit counters, and the CCPI2 has 8 64-bit counters.
>  */

Thanks.
>
> >  struct tx2_uncore_pmu {
> >       struct hlist_node hpnode;
> > @@ -69,12 +86,14 @@ struct tx2_uncore_pmu {
> >       int node;
> >       int cpu;
> >       u32 max_counters;
> > +     u32 counters_mask;
> >       u32 prorate_factor;
> >       u32 max_events;
> > +     u32 events_mask;
> >       u64 hrtimer_interval;
> >       void __iomem *base;
> >       DECLARE_BITMAP(active_counters, TX2_PMU_MAX_COUNTERS);
>
> This bitmap isn't big enough for the 4 new counters.
>
> > -     struct perf_event *events[TX2_PMU_MAX_COUNTERS];
> > +     struct perf_event **events;
>
> As above, can't we bump TX2_PMU_MAX_COUNTERS to 8 rather than making
> this a dynamic allocation?

events is only relevant for L3 and DMC since they use timer callbacks.
This is done as per previous review comments.

>
> [...]
>
> >  static inline u32 reg_readl(unsigned long addr)
> >  {
> >       return readl((void __iomem *)addr);
> >  }
> >
> > +static inline u32 reg_readq(unsigned long addr)
> > +{
> > +     return readq((void __iomem *)addr);
> > +}
>
> Presumably reg_readq() should return a u64.

Yes,  My bad.

>
> [...]
>
> > +static void uncore_start_event_ccpi2(struct perf_event *event, int flags)
> > +{
> > +     u32 emask;
> > +     struct hw_perf_event *hwc = &event->hw;
> > +     struct tx2_uncore_pmu *tx2_pmu;
> > +
> > +     tx2_pmu = pmu_to_tx2_pmu(event->pmu);
> > +     emask = tx2_pmu->events_mask;
> > +
> > +     /* Bit [09:00] to set event id, set level and type to 1 */
> > +     reg_writel((3 << 10) |
>
> Do you mean that bits [11:10] are level and type?

Yes, i will change the comment.
>
> What exactly are 'level' and 'type'?

They are for other settings which are not relevant for software/kernel.
>
> Can we give those bits mnemonics?
>
> > +                     GET_EVENTID(event, emask), hwc->config_base);
> > +     /* reset[4], enable[0] and start[1] counters */
>
> Rather than using magic numbers everywhere, please give these mnemonics,
> e.g.
>
> #define CCPI2_PERF_CTL_ENABLE   BIT(0)
> #define CCPI2_PERF_CTL_START    BIT(1)
> #define CCPI2_PERF_CTL_RESET    BIT(4)

not used everywhere, only in this function.
I can add these macros.

>
> > +     reg_writel(0x13, hwc->event_base + CCPI2_PERF_CTL);
>
> ... and then you can OR them in here:

OK
>
>         ctl = CCPI2_PERF_CTL_ENABLE |
>               CCPI2_PERF_CTL_START |
>               CCPI2_PERF_CTL_RESET;
>         reg_writel(ctl, hwc->event_base + CCPI2_PERF_CTL);
>
> [...]
>
> > @@ -456,8 +603,9 @@ static void tx2_uncore_event_start(struct perf_event *event, int flags)
> >       tx2_pmu->start_event(event, flags);
> >       perf_event_update_userpage(event);
> >
> > -     /* Start timer for first event */
> > -     if (bitmap_weight(tx2_pmu->active_counters,
> > +     /* Start timer for first non ccpi2 event */
> > +     if (tx2_pmu->type != PMU_TYPE_CCPI2 &&
> > +                     bitmap_weight(tx2_pmu->active_counters,
> >                               tx2_pmu->max_counters) == 1) {
> >               hrtimer_start(&tx2_pmu->hrtimer,
> >                       ns_to_ktime(tx2_pmu->hrtimer_interval),
>
> This would be easier to read as two statements:
>
>         /* No hrtimer needed with 64-bit counters */
>         if (tx2_pmu->type == PMU_TYPE_CCPI2)
>                 return;
>
>         /* Start timer for first event */
>         if (bitmap_weight(tx2_pmu->active_counters,
>             tx2_pmu->max_counters) != 1) {
>                 ...
>         }
>

OK, makes sense.

> > @@ -495,7 +643,8 @@ static int tx2_uncore_event_add(struct perf_event *event, int flags)
> >       if (hwc->idx < 0)
> >               return -EAGAIN;
> >
> > -     tx2_pmu->events[hwc->idx] = event;
> > +     if (tx2_pmu->events)
> > +             tx2_pmu->events[hwc->idx] = event;
>
> So this is NULL for CCPI2?

Yes.
>
> I guess we don't strictly need the if we don't have a hrtimer to update
> event counts, but this makes the code more complicated than it needs to
> be.

Yes I am using tx2_pmu->events to differentiate the type, it is NULL for CCPI2.
I can extend same to tx2_uncore_event_start().
>
> [...]
>
> > @@ -580,8 +732,12 @@ static int tx2_uncore_pmu_add_dev(struct tx2_uncore_pmu *tx2_pmu)
> >                       cpu_online_mask);
> >
> >       tx2_pmu->cpu = cpu;
> > -     hrtimer_init(&tx2_pmu->hrtimer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
> > -     tx2_pmu->hrtimer.function = tx2_hrtimer_callback;
> > +     /* CCPI2 counters are 64 bit counters, no overflow  */
> > +     if (tx2_pmu->type != PMU_TYPE_CCPI2) {
> > +             hrtimer_init(&tx2_pmu->hrtimer,
> > +                             CLOCK_MONOTONIC, HRTIMER_MODE_REL);
> > +             tx2_pmu->hrtimer.function = tx2_hrtimer_callback;
> > +     }
>
> Hmmm... this means that tx2_pmu->hrtimer.function is NULL for the CCPI2
> PMU. I think it would be best to check that when (re)programming the
> counters rather than the PMU type. For example, in
> tx2_uncore_event_start() we could have:
>
>         if (!tx2_pmu->hrtimer.function)
>                 return;
>         if (bitmap_weight(tx2_pmu->active_counters,
>             tx2_pmu->max_counters) != 1) {
>                 ...
>         }
>

Yes it is NULL for CCPI2.
Ok, I can use tx2_pmu->events instead(like other places).

> Thanks,
> Mark.

Thanks,
Ganapat

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

^ permalink raw reply

* Re: [PATCH 2/8] ARM: OMAP2+: Remove unconfigured midlemode for am3 lcdc
From: Tony Lindgren @ 2019-08-13 10:47 UTC (permalink / raw)
  To: Suman Anna
  Cc: Nishanth Menon, Tero Kristo, Dave Gerlach, Keerthy, linux-kernel,
	Jyri Sarha, Peter Ujfalusi, Faiz Abbas, Greg Kroah-Hartman,
	linux-omap, linux-arm-kernel, Roger Quadros
In-Reply-To: <d2414422-3ca5-4883-c94a-d3acdca70c87@ti.com>

* Suman Anna <s-anna@ti.com> [190724 18:29]:
> On 7/24/19 1:31 AM, Tony Lindgren wrote:
> > OK thanks for testing. Let's drop this for now, sounds like there is
> > some midlemode configuration happening even with no flags set.
> 
> You were dropping the ti,sysc-midle property in patch 8, is that still
> ok without this patch?

Yeah let's wait on that one too until we hear back from Jyri.

Regards,

Tony

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

^ permalink raw reply

* [PATCH 1/2] PCIe: imx6: imx7d: add support for internal phy refclk source
From: André Draszik @ 2019-08-13 10:37 UTC (permalink / raw)
  To: linux-kernel
  Cc: Lorenzo Pieralisi, Richard Zhu, André Draszik, Sascha Hauer,
	Fabio Estevam, NXP Linux Team, Pengutronix Kernel Team, linux-pci,
	Bjorn Helgaas, Shawn Guo, linux-arm-kernel, Lucas Stach

The i.MX7D variant of the IP can use either an external
crystal oscillator input or an internal clock input as
a reference clock input for the PCIe PHY.

Add support for an optional property 'fsl,pcie-phy-refclk-internal'
If present then the internal clock input is used as
PCIe PHY reference clock source. The previous default
of using an external ocsillator input (if the property
doesn't exist), doesn't change.

Signed-off-by: André Draszik <git@andred.net>
Cc: Richard Zhu <hongxing.zhu@nxp.com>
Cc: Lucas Stach <l.stach@pengutronix.de>
Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Shawn Guo <shawnguo@kernel.org>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
Cc: Fabio Estevam <festevam@gmail.com>
Cc: NXP Linux Team <linux-imx@nxp.com>
Cc: linux-pci@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/pci/controller/dwc/pci-imx6.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index 9b5cb5b70389..bb3700c9157c 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -63,6 +63,7 @@ struct imx6_pcie {
 	struct dw_pcie		*pci;
 	int			reset_gpio;
 	bool			gpio_active_high;
+	bool			phy_refclk_internal;
 	struct clk		*pcie_bus;
 	struct clk		*pcie_phy;
 	struct clk		*pcie_inbound_axi;
@@ -635,7 +636,10 @@ static void imx6_pcie_init_phy(struct imx6_pcie *imx6_pcie)
 		break;
 	case IMX7D:
 		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
-				   IMX7D_GPR12_PCIE_PHY_REFCLK_SEL, 0);
+				   IMX7D_GPR12_PCIE_PHY_REFCLK_SEL,
+				   imx6_pcie->phy_refclk_internal
+				   ? IMX7D_GPR12_PCIE_PHY_REFCLK_SEL
+				   : 0);
 		break;
 	case IMX6SX:
 		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
@@ -1171,6 +1175,9 @@ static int imx6_pcie_probe(struct platform_device *pdev)
 	if (ret)
 		imx6_pcie->link_gen = 1;
 
+	imx6_pcie->phy_refclk_internal =
+		of_property_read_bool(node, "fsl,pcie-phy-refclk-internal");
+
 	imx6_pcie->vpcie = devm_regulator_get_optional(&pdev->dev, "vpcie");
 	if (IS_ERR(imx6_pcie->vpcie)) {
 		if (PTR_ERR(imx6_pcie->vpcie) == -EPROBE_DEFER)
-- 
2.23.0.rc1


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

^ permalink raw reply related

* [PATCH 2/2] dt-bindings: imx6q-pcie: add "fsl, pcie-phy-refclk-internal" for i.MX7D
From: André Draszik @ 2019-08-13 10:37 UTC (permalink / raw)
  To: linux-kernel
  Cc: Mark Rutland, devicetree, Fabio Estevam, Richard Zhu,
	André Draszik, Sascha Hauer, Rob Herring, NXP Linux Team,
	Pengutronix Kernel Team, linux-pci, Bjorn Helgaas, Shawn Guo,
	linux-arm-kernel, Lucas Stach
In-Reply-To: <20190813103759.38358-1-git@andred.net>

The i.MX7D variant of the IP can use either an external
crystal oscillator input or an internal clock input as
a reference clock input for the PCIe PHY.

Document the optional property 'fsl,pcie-phy-refclk-internal'

Signed-off-by: André Draszik <git@andred.net>
Cc: Richard Zhu <hongxing.zhu@nxp.com>
Cc: Lucas Stach <l.stach@pengutronix.de>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Shawn Guo <shawnguo@kernel.org>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
Cc: Fabio Estevam <festevam@gmail.com>
Cc: NXP Linux Team <linux-imx@nxp.com>
Cc: linux-pci@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: devicetree@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
index a7f5f5afa0e6..985d7083df9f 100644
--- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
+++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
@@ -56,6 +56,11 @@ Additional required properties for imx7d-pcie and imx8mq-pcie:
 	       - "turnoff"
 - fsl,imx7d-pcie-phy: A phandle to an fsl,imx7d-pcie-phy node.
 
+Additional optional properties for imx7d-pcie:
+- fsl,pcie-phy-refclk-internal: If present then an internal PLL input is used
+  as PCIe PHY reference clock source. By default an external ocsillator input
+  is used.
+
 Additional required properties for imx8mq-pcie:
 - clock-names: Must include the following additional entries:
 	- "pcie_aux"
-- 
2.23.0.rc1


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

^ permalink raw reply related

* Re: [EXT] Re: [PATCH 00/10] Improvements and fixes for mxsfb DRM driver
From: Robert Chiras @ 2019-08-13 10:36 UTC (permalink / raw)
  To: agx@sigxcpu.org
  Cc: marex@denx.de, devicetree@vger.kernel.org, kernel@pengutronix.de,
	airlied@linux.ie, shawnguo@kernel.org,
	dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org,
	stefan@agner.ch, robh+dt@kernel.org, dl-linux-imx,
	daniel@ffwll.ch, mark.rutland@arm.com, festevam@gmail.com,
	s.hauer@pengutronix.de, linux-arm-kernel@lists.infradead.org
In-Reply-To: <20190813102307.GA22623@bogon.m.sigxcpu.org>

On Ma, 2019-08-13 at 12:23 +0200, Guido Günther wrote:
> Hi Robert,
> On Wed, Jun 26, 2019 at 04:32:08PM +0300, Robert Chiras wrote:
> > 
> > This patch-set improves the use of eLCDIF block on iMX 8 SoCs (like
> > 8MQ, 8MM
> > and 8QXP). Following, are the new features added and fixes from
> > this
> > patch-set:
> There was some feedback on various patches, do you intend to pick
> that
> up again? That would be cool since there's some overlapping work
> popping
> up already e.g. in
> 
>     https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2
> Fpatchwork.freedesktop.org%2Fseries%2F64595%2F&amp;data=02%7C01%7Crob
> ert.chiras%40nxp.com%7C6ca6724a656b41912f0408d71fd83da0%7C686ea1d3bc2
> b4c6fa92cd99c5c301635%7C0%7C0%7C637012885918631196&amp;sdata=b3CrbNu%
> 2FcsWBOA%2BcaQLX%2BrlrK7%2Fhf2%2F1vZS3eQGN7aM%3D&amp;reserved=0
> 
> showing up and it's the base for the tiny
> 
>     https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2
> Fpatchwork.freedesktop.org%2Fseries%2F64300%2F&amp;data=02%7C01%7Crob
> ert.chiras%40nxp.com%7C6ca6724a656b41912f0408d71fd83da0%7C686ea1d3bc2
> b4c6fa92cd99c5c301635%7C0%7C0%7C637012885918641196&amp;sdata=h6KLVnSx
> xBwvK%2FvPF9zt4DQR6WnF1pyQSwKBTO4rQTg%3D&amp;reserved=0
> 
> Cheers,
>  -- Guido

Hi Guido,

Yes, I plan to submit a next revision, but first I wanted to try it with your patch-set for the nwl-dsi driver.
Thanks for the heads-up.

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

^ permalink raw reply

* Re: [PATCH 14/22] ARM: omap1: use pci_ioremap_io() for omap_cf
From: Tony Lindgren @ 2019-08-13 10:36 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Aaro Koskinen, Greg Kroah-Hartman, Linus Walleij,
	Bartlomiej Zolnierkiewicz, linux-kernel, Dominik Brodowski,
	Tomi Valkeinen, linux-omap, linux-arm-kernel
In-Reply-To: <20190808212234.2213262-15-arnd@arndb.de>

Hi,

* Arnd Bergmann <arnd@arndb.de> [190808 21:34]:
> The ISA I/O space handling in omap_cf is incompatible with
> PCI drivers in a multiplatform kernel, and requires a custom
> mach/io.h.
> 
> Change the driver to use pci_ioremap_io() like PCI drivers do,
> so the generic ioport access can work across platforms.
> 
> To actually use that code, we have to select CONFIG_PCI
> here.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Looks like this series boots for me on 5912osk up to this
patch, but this patch breaks booting somehow.

Any ideas for fixes?

Regards,

Tony

> ---
>  arch/arm/Kconfig                      |  2 +-
>  arch/arm/mach-omap1/include/mach/io.h | 45 ---------------------------
>  drivers/pcmcia/omap_cf.c              |  9 ++----
>  3 files changed, 4 insertions(+), 52 deletions(-)
>  delete mode 100644 arch/arm/mach-omap1/include/mach/io.h
> 
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index b7162ac8d756..8263fe7a5e64 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -496,13 +496,13 @@ config ARCH_OMAP1
>  	select ARCH_OMAP
>  	select CLKDEV_LOOKUP
>  	select CLKSRC_MMIO
> +	select FORCE_PCI if PCCARD
>  	select GENERIC_CLOCKEVENTS
>  	select GENERIC_IRQ_CHIP
>  	select GENERIC_IRQ_MULTI_HANDLER
>  	select GPIOLIB
>  	select HAVE_IDE
>  	select IRQ_DOMAIN
> -	select NEED_MACH_IO_H if PCCARD
>  	select NEED_MACH_MEMORY_H
>  	select SPARSE_IRQ
>  	help
> diff --git a/arch/arm/mach-omap1/include/mach/io.h b/arch/arm/mach-omap1/include/mach/io.h
> deleted file mode 100644
> index ce4f8005b26f..000000000000
> --- a/arch/arm/mach-omap1/include/mach/io.h
> +++ /dev/null
> @@ -1,45 +0,0 @@
> -/*
> - * arch/arm/mach-omap1/include/mach/io.h
> - *
> - * IO definitions for TI OMAP processors and boards
> - *
> - * Copied from arch/arm/mach-sa1100/include/mach/io.h
> - * Copyright (C) 1997-1999 Russell King
> - *
> - * This program is free software; you can redistribute it and/or modify it
> - * under the terms of the GNU General Public License as published by the
> - * Free Software Foundation; either version 2 of the License, or (at your
> - * option) any later version.
> - *
> - * THIS SOFTWARE IS PROVIDED ``AS IS'' AND ANY EXPRESS OR IMPLIED
> - * WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF
> - * MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN
> - * NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT,
> - * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
> - * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
> - * USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON
> - * ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> - * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
> - * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> - *
> - * You should have received a copy of the GNU General Public License along
> - * with this program; if not, write to the Free Software Foundation, Inc.,
> - * 675 Mass Ave, Cambridge, MA 02139, USA.
> - *
> - * Modifications:
> - *  06-12-1997	RMK	Created.
> - *  07-04-1999	RMK	Major cleanup
> - */
> -
> -#ifndef __ASM_ARM_ARCH_IO_H
> -#define __ASM_ARM_ARCH_IO_H
> -
> -#define IO_SPACE_LIMIT 0xffffffff
> -
> -/*
> - * We don't actually have real ISA nor PCI buses, but there is so many
> - * drivers out there that might just work if we fake them...
> - */
> -#define __io(a)		__typesafe_io(a)
> -
> -#endif
> diff --git a/drivers/pcmcia/omap_cf.c b/drivers/pcmcia/omap_cf.c
> index 98df6473034d..9f8ad82f5fce 100644
> --- a/drivers/pcmcia/omap_cf.c
> +++ b/drivers/pcmcia/omap_cf.c
> @@ -235,9 +235,9 @@ static int __init omap_cf_probe(struct platform_device *pdev)
>  	cf->phys_cf = res->start;
>  
>  	/* pcmcia layer only remaps "real" memory */
> -	cf->socket.io_offset = (unsigned long)
> -			ioremap(cf->phys_cf + SZ_4K, SZ_2K);
> -	if (!cf->socket.io_offset)
> +	cf->socket.io_offset = 0x10000;
> +	status = pci_ioremap_io(cf->socket.io_offset, cf->phys_cf + SZ_4K);
> +	if (status)
>  		goto fail1;
>  
>  	if (!request_mem_region(cf->phys_cf, SZ_8K, driver_name))
> @@ -281,8 +281,6 @@ static int __init omap_cf_probe(struct platform_device *pdev)
>  fail2:
>  	release_mem_region(cf->phys_cf, SZ_8K);
>  fail1:
> -	if (cf->socket.io_offset)
> -		iounmap((void __iomem *) cf->socket.io_offset);
>  	free_irq(irq, cf);
>  fail0:
>  	kfree(cf);
> @@ -296,7 +294,6 @@ static int __exit omap_cf_remove(struct platform_device *pdev)
>  	cf->active = 0;
>  	pcmcia_unregister_socket(&cf->socket);
>  	del_timer_sync(&cf->timer);
> -	iounmap((void __iomem *) cf->socket.io_offset);
>  	release_mem_region(cf->phys_cf, SZ_8K);
>  	free_irq(cf->irq, cf);
>  	kfree(cf);
> -- 
> 2.20.0
> 

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

^ permalink raw reply

* Re: [PATCH for v5.3] ARM: OMAP1: ams-delta-fiq: Fix missing irq_ack
From: Tony Lindgren @ 2019-08-13 10:26 UTC (permalink / raw)
  To: Janusz Krzysztofik
  Cc: linux-omap, linux-kernel, linux-arm-kernel, Aaro Koskinen
In-Reply-To: <20190811084802.630-1-jmkrzyszt@gmail.com>

* Janusz Krzysztofik <jmkrzyszt@gmail.com> [190811 01:48]:
> Non-serio path of Amstrad Delta FIQ deferred handler depended on
> irq_ack() method provided by OMAP GPIO driver.  That method has been
> removed by commit 693de831c6e5 ("gpio: omap: remove irq_ack method").
> Remove useless code from the deferred handler and reimplement the
> missing operation inside the base FIQ handler.
> 
> Should another dependency - irq_unmask() - be ever removed from the OMAP
> GPIO driver, WARN once if missing.

Thanks applying into fixes.

Tony

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

^ permalink raw reply

* Re: [alsa-devel] Regression in next with codec unload and snd_soc_component_get/put
From: Tony Lindgren @ 2019-08-13 10:24 UTC (permalink / raw)
  To: Takashi Iwai
  Cc: alsa-devel, Kuninori Morimoto, linux-kernel, Jaroslav Kysela,
	Mark Brown, linux-omap, linux-arm-kernel
In-Reply-To: <s5hv9v6hhm9.wl-tiwai@suse.de>

* Takashi Iwai <tiwai@suse.de> [190809 08:24]:
> On Fri, 09 Aug 2019 09:46:43 +0200,
> Tony Lindgren wrote:
> > 
> > * Takashi Iwai <tiwai@suse.de> [190809 07:25]:
> > > On Fri, 09 Aug 2019 09:00:03 +0200,
> > > Tony Lindgren wrote:
> > > > 
> > > > Hi,
> > > > 
> > > > Looks like commit 4a81e8f30d0b ("ASoC: soc-component: add
> > > > snd_soc_component_get/put()") causes a regression where trying
> > > > to rmmod a codec driver fails with module is in use error after
> > > > rmmod of snd-soc-audio-graph-card for example.
> > > > 
> > > > Any ideas what goes wrong there?
> > > 
> > > There is an obvious typo: soc_cleanup_component() should call
> > > snd_soc_component_module_put_when_remove() instead of *_close().
> > 
> > Making that change locally seems to fix it thanks.
> 
> ... and it was already fixed in the later commit 0e36f36b04e7
> "ASoC: soc-core: fix module_put() warning in soc_cleanup_component".

Mark, looks like this commit is still not in Linux next, forgot
to push out something?

Regards,

Tony

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

^ permalink raw reply

* Re: [PATCH 00/10] Improvements and fixes for mxsfb DRM driver
From: Guido Günther @ 2019-08-13 10:23 UTC (permalink / raw)
  To: Robert Chiras
  Cc: Marek Vasut, Mark Rutland, Pengutronix Kernel Team, devicetree,
	David Airlie, Fabio Estevam, Sascha Hauer, linux-kernel,
	Stefan Agner, Rob Herring, dri-devel, Daniel Vetter, Shawn Guo,
	linux-arm-kernel, NXP Linux Team
In-Reply-To: <1561555938-21595-1-git-send-email-robert.chiras@nxp.com>

Hi Robert,
On Wed, Jun 26, 2019 at 04:32:08PM +0300, Robert Chiras wrote:
> This patch-set improves the use of eLCDIF block on iMX 8 SoCs (like 8MQ, 8MM
> and 8QXP). Following, are the new features added and fixes from this
> patch-set:

There was some feedback on various patches, do you intend to pick that
up again? That would be cool since there's some overlapping work popping
up already e.g. in

    https://patchwork.freedesktop.org/series/64595/

showing up and it's the base for the tiny

    https://patchwork.freedesktop.org/series/64300/    

Cheers,
 -- Guido

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

^ permalink raw reply

* Re: [PATCH v2 1/3] arm64: imx8mq: add imx8mq iomux-gpr field defines
From: Guido Günther @ 2019-08-13 10:10 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Mark Rutland, DTML, Jernej Skrabec, Pengutronix Kernel Team,
	Sam Ravnborg, Neil Armstrong, David Airlie, Fabio Estevam,
	Sascha Hauer, Jonas Karlman, Linux Kernel Mailing List, dri-devel,
	Andrzej Hajda, Rob Herring, NXP Linux Team, Daniel Vetter,
	Robert Chiras, Lee Jones, Shawn Guo, Linux ARM, Laurent Pinchart
In-Reply-To: <CAK8P3a3Vrd+sttJrQwD-jA9p_egG4x-hc41eGK8H-_aVm-uoYw@mail.gmail.com>

Hi Arnd,
On Tue, Aug 13, 2019 at 10:08:44AM +0200, Arnd Bergmann wrote:
> On Fri, Aug 9, 2019 at 6:24 PM Guido Günther <agx@sigxcpu.org> wrote:
> >
> > This adds all the gpr registers and the define needed for selecting
> > the input source in the imx-nwl drm bridge.
> >
> > Signed-off-by: Guido Günther <agx@sigxcpu.org>
> > +
> > +#define IOMUXC_GPR0    0x00
> > +#define IOMUXC_GPR1    0x04
> > +#define IOMUXC_GPR2    0x08
> > +#define IOMUXC_GPR3    0x0c
> > +#define IOMUXC_GPR4    0x10
> > +#define IOMUXC_GPR5    0x14
> > +#define IOMUXC_GPR6    0x18
> > +#define IOMUXC_GPR7    0x1c
> (more of the same)
> 
> huh?

These are the names from the imx8MQ reference manual (general purpose
registers, they lump together all sorts of things), it's the same on
imx6/imx7):

    https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/linux/mfd/syscon/imx6q-iomuxc-gpr.h
    https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/linux/mfd/syscon/imx7-iomuxc-gpr.h     

> > +/* i.MX8Mq iomux gpr register field defines */
> > +#define IMX8MQ_GPR13_MIPI_MUX_SEL              BIT(2)
> 
> I think this define should probably be local to the pinctrl driver, to
> ensure that no other drivers fiddle with the registers manually.

The purpose of these bits is for a driver to fiddle with them to select
the input source. Similar on imx7 it's already used for e.g. the phy
refclk in the pci controller:

    https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/pci/controller/dwc/pci-imx6.c#n638

The GPRs are not about pad configuration but gather all sorts of things
(section 8.2.4 of the imx8mq reference manual): pcie setup, dsi related
bits so I don't think this should be done via a pinctrl
driver. Should we handle that differently than on imx6/7?

Cheers,
 -- Guido

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

^ permalink raw reply

* Re: [PATCH v2 2/3] dt-bindings: display/bridge: Add binding for NWL mipi dsi host controller
From: Guido Günther @ 2019-08-13 10:10 UTC (permalink / raw)
  To: Rob Herring
  Cc: Mark Rutland, devicetree, Jernej Skrabec, Daniel Vetter,
	Sam Ravnborg, Neil Armstrong, David Airlie, Fabio Estevam,
	Sascha Hauer, Jonas Karlman, linux-kernel@vger.kernel.org,
	dri-devel, Andrzej Hajda, NXP Linux Team, Pengutronix Kernel Team,
	Robert Chiras, Lee Jones, Shawn Guo,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	Laurent Pinchart
In-Reply-To: <CAL_JsqK-5=WMZgNuJDTJ3Dm3YOJNw_9QCrPOOSe7MQzMV26pHw@mail.gmail.com>

Hi Rob,
thanks for having a look!

On Fri, Aug 09, 2019 at 02:41:03PM -0600, Rob Herring wrote:
> On Fri, Aug 9, 2019 at 10:24 AM Guido Günther <agx@sigxcpu.org> wrote:
> >
> > The Northwest Logic MIPI DSI IP core can be found in NXPs i.MX8 SoCs.
> >
> > Signed-off-by: Guido Günther <agx@sigxcpu.org>
> > ---
> >  .../bindings/display/bridge/nwl-dsi.yaml      | 155 ++++++++++++++++++
> >  1 file changed, 155 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/display/bridge/nwl-dsi.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/display/bridge/nwl-dsi.yaml b/Documentation/devicetree/bindings/display/bridge/nwl-dsi.yaml
> > new file mode 100644
> > index 000000000000..5ed8bc4a4d18
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/display/bridge/nwl-dsi.yaml
> > @@ -0,0 +1,155 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/display/bridge/imx-nwl-dsi.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Northwest Logic MIPI-DSI on imx SoCs
> > +
> > +maintainers:
> > +  - Guido Gúnther <agx@sigxcpu.org>
> > +  - Robert Chiras <robert.chiras@nxp.com>
> > +
> > +description: |
> > +  NWL MIPI-DSI host controller found on i.MX8 platforms. This is a dsi bridge for
> > +  the SOCs NWL MIPI-DSI host controller.
> > +
> > +properties:
> > +  compatible:
> > +    oneOf:
> > +      - items:
> > +        - const: fsl,imx8mq-nwl-dsi
> 
> Don't need oneOf nor items here for a single possible value:

I wanted to prepare for adding other SoCs so there's less diff noise
(other imx8 SoCs will be rather simple) but let's go with 'const' for
now then.

> compatible:
>   const: fsl,imx8mq-nwl-dsi
> 
> Or go ahead and add other compatibles because the 'if' below seems to
> indicate you'll have more.

> > +  reg:
> > +    maxItems: 1
> > +
> > +  interrupts:
> > +    maxItems: 1
> > +
> > +  clocks:
> > +    items:
> > +      - description: DSI core clock
> > +      - description: RX_ESC clock (used in escape mode)
> > +      - description: TX_ESC clock (used in escape mode)
> > +      - description: PHY_REF clock
> > +
> > +  clock-names:
> > +    items:
> > +      - const: core
> > +      - const: rx_esc
> > +      - const: tx_esc
> > +      - const: phy_ref
> > +
> > +  phys:
> > +    maxItems: 1
> > +    description:
> > +      A phandle to the phy module representing the DPHY
> > +
> > +  phy-names:
> > +    items:
> > +      - const: dphy
> > +
> > +  power-domains:
> > +    maxItems: 1
> > +    description:
> > +      A phandle to the power domain
> > +
> > +  resets:
> > +    maxItems: 4
> > +    description:
> > +      A phandle to the reset controller
> 
> Sounds like 4 phandles... This should look similar to 'clocks'.

Added them individually, will be soc specific too later on.

> 
> > +
> > +  reset-names:
> > +    items:
> > +      - const: byte
> > +      - const: dpi
> > +      - const: esc
> > +      - const: pclk
> > +
> > +  mux-sel:
> 
> Needs a vendor prefix and will need a $ref to the type.

Made that fsl,mux-sel. This require me to add '$ref:
/schemas/types.yaml#definitions/phandle' as well which
I hope is correct.

> > +    maxItems: 1
> > +    description:
> > +      A phandle to the MUX register set
> > +
> > +  port:
> > +    type: object
> > +    description:
> > +      A input put or output port node.
> > +
> > +  ports:
> > +    type: object
> > +    description:
> > +      A node containing DSI input & output port nodes with endpoint
> > +      definitions as documented in
> > +      Documentation/devicetree/bindings/graph.txt.
> 
> You need to define what port@0 and port@1 are.

Added.

> 
> > +
> > +patternProperties:
> > +  "^panel@[0-9]+$": true
> > +
> > +allOf:
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            enum:
> > +              - fsl,imx8mq-nwl-dsi
> 
> This conditional isn't needed until you have more than one compatible.

Again intended for other upcoming SoCs but dropped for now.

> > +      required:
> > +        - resets
> > +        - reset-names
> > +        - mux-sel
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - interrupts
> > +  - clocks
> > +  - clock-names
> > +  - phys
> > +  - phy-names
> 
> ports should be required.

Added.

> > +
> > +examples:
> > + - |
> > +
> > +   mipi_dsi: mipi_dsi@30a00000 {
> > +              #address-cells = <1>;
> > +              #size-cells = <0>;
> > +              compatible = "fsl,imx8mq-nwl-dsi";
> > +              reg = <0x30A00000 0x300>;
> > +              clocks = <&clk 163>, <&clk 244>, <&clk 245>, <&clk 164>;
> > +              clock-names = "core", "rx_esc", "tx_esc", "phy_ref";
> > +              interrupts = <0 34 4>;
> > +              power-domains = <&pgc_mipi>;
> > +              resets = <&src 0>, <&src 1>, <&src 2>, <&src 3>;
> > +              reset-names = "byte", "dpi", "esc", "pclk";
> > +              mux-sel = <&iomuxc_gpr>;
> > +              phys = <&dphy>;
> > +              phy-names = "dphy";
> > +
> > +              panel@0 {
> > +                      compatible = "...";
> 
> Needs to be a valid compatible. Also need 'reg' here or drop the
> unit-address.

Fixed.

> 
> 
> > +                      port@0 {
> > +                           panel_in: endpoint {
> > +                                     remote-endpoint = <&mipi_dsi_out>;
> > +                           };
> > +                      };
> > +              };
> > +
> > +              ports {
> > +                    #address-cells = <1>;
> > +                    #size-cells = <0>;
> > +
> > +                    port@0 {
> > +                           reg = <0>;
> > +                           mipi_dsi_in: endpoint {
> > +                                        remote-endpoint = <&lcdif_mipi_dsi>;
> > +                           };
> > +                    };
> > +                    port@1 {
> > +                           reg = <1>;
> > +                           mipi_dsi_out: endpoint {
> > +                                         remote-endpoint = <&panel_in>;
> > +                           };
> > +                    };
> > +              };
> > +      };
> > --
> > 2.20.1
> >
> 

Cheers and thanks again for having a look!
 -- Guido

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

^ permalink raw reply

* Re: [PATCHv5 1/2] PCI: layerscape: Add the bar_fixed_64bit property in EP driver.
From: Lorenzo Pieralisi @ 2019-08-13 10:04 UTC (permalink / raw)
  To: Xiaowei Bao
  Cc: linux-arm-kernel, roy.zang, leonard.crestez, hayashi.kunihiko,
	andrew.smirnov, linux-pci, linux-kernel, yue.wang, kishon,
	minghuan.Lian, dwmw, jonnyc, bhelgaas, tpiepho, linuxppc-dev,
	mingkai.hu, l.stach
In-Reply-To: <20190813062840.2733-1-xiaowei.bao@nxp.com>

git log --oneline --follow drivers/pci/controller/dwc/pci-layerscape.c

Do you see any commit with a $SUBJECT ending with a period ?

There is not. So remove it from yours too.

On Tue, Aug 13, 2019 at 02:28:39PM +0800, Xiaowei Bao wrote:
> The PCIe controller of layerscape just have 4 BARs, BAR0 and BAR1
> is 32bit, BAR2 and BAR4 is 64bit, this is determined by hardware,
> so set the bar_fixed_64bit with 0x14.
> 
> Signed-off-by: Xiaowei Bao <xiaowei.bao@nxp.com>
> ---
> v2:
>  - Replace value 0x14 with a macro.
> v3:
>  - No change.
> v4:
>  - send the patch again with '--to'.
> v5:
>  - fix the commit message.
> 
>  drivers/pci/controller/dwc/pci-layerscape-ep.c | 1 +
>  1 file changed, 1 insertion(+)

scripts/get_maintainer.pl -f drivers/pci/controller/dwc/pci-layerscape-ep.c
Now, with the output you get justify all the people you send this email
to.

So, again, trim the CC list and it is the last time I tell you.

Before sending patches on mailing lists use git --dry-run to check
the emails you are sending.

Thanks,
Lorenzo

> diff --git a/drivers/pci/controller/dwc/pci-layerscape-ep.c b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> index be61d96..ca9aa45 100644
> --- a/drivers/pci/controller/dwc/pci-layerscape-ep.c
> +++ b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> @@ -44,6 +44,7 @@ static const struct pci_epc_features ls_pcie_epc_features = {
>  	.linkup_notifier = false,
>  	.msi_capable = true,
>  	.msix_capable = false,
> +	.bar_fixed_64bit = (1 << BAR_2) | (1 << BAR_4),
>  };
>  
>  static const struct pci_epc_features*
> -- 
> 2.9.5
> 

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

^ permalink raw reply

* Re: [PATCH 2/2] arm64: Don't use KPTI where we have E0PD
From: Suzuki K Poulose @ 2019-08-13 10:01 UTC (permalink / raw)
  To: broonie, catalin.marinas, will; +Cc: linux-arm-kernel
In-Reply-To: <20190812125738.17388-3-broonie@kernel.org>



On 12/08/2019 13:57, Mark Brown wrote:
> Since E0PD is intended to fulfil the same role as KPTI we don't need to
> use KPTI on CPUs where E0PD is available, we can rely on E0PD instead.
> Change the check that forces KPTI on when KASLR is enabled to check for
> E0PD before doing so, CPUs with E0PD are not expected to be affected by
> meltdown so should not need to enable KPTI for other reasons.
> 
> Since we repeat the KPTI check for all CPUs we will still enable KPTI if
> any of the CPUs in the system lacks E0PD. Since KPTI itself is not changed
> by this patch once we enable KPTI we will do so for all CPUs. This is safe
> but not optimally performant for such systems.
> 
> KPTI can still be forced on from the command line if required.
> 
> Signed-off-by: Mark Brown <broonie@kernel.org>
> ---
>   arch/arm64/kernel/cpufeature.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
> index 4aa1d2026bef..322004409211 100644
> --- a/arch/arm64/kernel/cpufeature.c
> +++ b/arch/arm64/kernel/cpufeature.c
> @@ -995,7 +995,7 @@ static bool unmap_kernel_at_el0(const struct arm64_cpu_capabilities *entry,
>   
>   	/* Useful for KASLR robustness */
>   	if (IS_ENABLED(CONFIG_RANDOMIZE_BASE) && kaslr_offset() > 0) {
> -		if (!__kpti_forced) {
> +		if (!__kpti_forced && !this_cpu_has_cap(ARM64_HAS_E0PD)) {
>   			str = "KASLR";
>   			__kpti_forced = 1;
>   		}

Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>

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

^ permalink raw reply

* [PATCH V5 3/3] mmc: mmci: sdmmc: add busy_complete callback
From: Ludovic Barre @ 2019-08-13  9:59 UTC (permalink / raw)
  To: Ulf Hansson, Rob Herring
  Cc: devicetree, Alexandre Torgue, linux-mmc, linux-kernel,
	srinivas.kandagatla, Ludovic Barre, Maxime Coquelin, linux-stm32,
	linux-arm-kernel
In-Reply-To: <20190813095951.26275-1-ludovic.Barre@st.com>

From: Ludovic Barre <ludovic.barre@st.com>

This patch adds a specific busy_complete callback for sdmmc variant.

sdmmc has 2 status flags:
-busyd0: This is a hardware status flag (inverted value of d0 line).
it does not generate an interrupt.
-busyd0end: This indicates only end of busy following a CMD response.
On busy to Not busy changes, an interrupt is generated (if unmask)
and BUSYD0END status flag is set. Status flag is cleared by writing
corresponding interrupt clear bit in MMCICLEAR.

The legacy busy completion monitors step by step the busy progression
start/in-progress/end. On sdmmc variant, the monitoring of busy steps
is difficult and not adapted (the software can miss a step and locks
the monitoring), the sdmmc has just need to wait the busyd0end bit
without monitoring all the changes.

Signed-off-by: Ludovic Barre <ludovic.barre@st.com>
---
 drivers/mmc/host/mmci.c             |  3 +++
 drivers/mmc/host/mmci.h             |  1 +
 drivers/mmc/host/mmci_stm32_sdmmc.c | 38 +++++++++++++++++++++++++++++
 3 files changed, 42 insertions(+)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 9eac3f482119..9bec82d2dbf7 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -260,6 +260,9 @@ static struct variant_data variant_stm32_sdmmc = {
 	.datalength_bits	= 25,
 	.datactrl_blocksz	= 14,
 	.stm32_idmabsize_mask	= GENMASK(12, 5),
+	.busy_timeout		= true,
+	.busy_detect_flag	= MCI_STM32_BUSYD0,
+	.busy_detect_mask	= MCI_STM32_BUSYD0ENDMASK,
 	.init			= sdmmc_variant_init,
 };
 
diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h
index 733f9a035b06..841c5281beb5 100644
--- a/drivers/mmc/host/mmci.h
+++ b/drivers/mmc/host/mmci.h
@@ -164,6 +164,7 @@
 #define MCI_ST_CARDBUSY		(1 << 24)
 /* Extended status bits for the STM32 variants */
 #define MCI_STM32_BUSYD0	BIT(20)
+#define MCI_STM32_BUSYD0END	BIT(21)
 
 #define MMCICLEAR		0x038
 #define MCI_CMDCRCFAILCLR	(1 << 0)
diff --git a/drivers/mmc/host/mmci_stm32_sdmmc.c b/drivers/mmc/host/mmci_stm32_sdmmc.c
index 8e83ae6920ae..bb5499cc9e81 100644
--- a/drivers/mmc/host/mmci_stm32_sdmmc.c
+++ b/drivers/mmc/host/mmci_stm32_sdmmc.c
@@ -282,6 +282,43 @@ static u32 sdmmc_get_dctrl_cfg(struct mmci_host *host)
 	return datactrl;
 }
 
+bool sdmmc_busy_complete(struct mmci_host *host, u32 status, u32 err_msk)
+{
+	void __iomem *base = host->base;
+	u32 busy_d0, busy_d0end, mask;
+
+	mask = readl_relaxed(base + MMCIMASK0);
+	busy_d0end = readl_relaxed(base + MMCISTATUS) & MCI_STM32_BUSYD0END;
+	busy_d0 = readl_relaxed(base + MMCISTATUS) & MCI_STM32_BUSYD0;
+
+	/* complete if there is an error or busy_d0end */
+	if ((status & err_msk) || busy_d0end)
+		goto complete;
+
+	/*
+	 * On response the busy signaling is reflected in the BUSYD0 flag.
+	 * if busy_d0 is in-progress we must activate busyd0end interrupt
+	 * to wait this completion. Else this request has no busy step.
+	 */
+	if (busy_d0) {
+		if (!host->busy_status) {
+			writel_relaxed(mask | host->variant->busy_detect_mask,
+				       base + MMCIMASK0);
+			host->busy_status = status &
+				(MCI_CMDSENT | MCI_CMDRESPEND);
+		}
+		return false;
+	}
+
+complete:
+	writel_relaxed(mask & ~host->variant->busy_detect_mask,
+		       base + MMCIMASK0);
+	writel_relaxed(host->variant->busy_detect_mask, base + MMCICLEAR);
+	host->busy_status = 0;
+
+	return true;
+}
+
 static struct mmci_host_ops sdmmc_variant_ops = {
 	.validate_data = sdmmc_idma_validate_data,
 	.prep_data = sdmmc_idma_prep_data,
@@ -292,6 +329,7 @@ static struct mmci_host_ops sdmmc_variant_ops = {
 	.dma_finalize = sdmmc_idma_finalize,
 	.set_clkreg = mmci_sdmmc_set_clkreg,
 	.set_pwrreg = mmci_sdmmc_set_pwrreg,
+	.busy_complete = sdmmc_busy_complete,
 };
 
 void sdmmc_variant_init(struct mmci_host *host)
-- 
2.17.1


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

^ permalink raw reply related

* [PATCH V5 0/3] mmc: mmci: add busy detect for stm32 sdmmc variant
From: Ludovic Barre @ 2019-08-13  9:59 UTC (permalink / raw)
  To: Ulf Hansson, Rob Herring
  Cc: devicetree, Alexandre Torgue, linux-mmc, linux-kernel,
	srinivas.kandagatla, Ludovic Barre, Maxime Coquelin, linux-stm32,
	linux-arm-kernel

From: Ludovic Barre <ludovic.barre@st.com>

This patch series adds busy detect for stm32 sdmmc variant.
Some adaptations are required:
-On sdmmc the data timer is started on data transfert
and busy state, so we must add hardware busy timeout support.
-Add busy_complete callback at mmci_host_ops to allow to define
a specific busy completion by variant.
-Add sdmmc busy_complete calback.

V5:
-Replaces !cmd->data to !host->mrq->data to avoid overwrite
 of datatimer register by the first command (cmd23, without data) of
 SBC request.

V4:
-Re-work with busy_complete callback
-In series, move "mmc: mmci: add hardware busy timeout feature" in
first to simplify busy_complete prototype with err_msk parameter.

V3:
-rebase on latest mmc next
-replace re-read by status parameter. 

V2:
-mmci_cmd_irq cleanup in separate patch.
-simplify the busy_detect_flag exclude
-replace sdmmc specific comment in
"mmc: mmci: avoid fake busy polling in mmci_irq"
to focus on common behavior

Ludovic Barre (3):
  mmc: mmci: add hardware busy timeout feature
  mmc: mmci: add busy_complete callback
  mmc: mmci: sdmmc: add busy_complete callback

 drivers/mmc/host/mmci.c             | 178 +++++++++++++++++-----------
 drivers/mmc/host/mmci.h             |   7 +-
 drivers/mmc/host/mmci_stm32_sdmmc.c |  38 ++++++
 3 files changed, 151 insertions(+), 72 deletions(-)

-- 
2.17.1


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

^ permalink raw reply

* [PATCH V5 2/3] mmc: mmci: add busy_complete callback
From: Ludovic Barre @ 2019-08-13  9:59 UTC (permalink / raw)
  To: Ulf Hansson, Rob Herring
  Cc: devicetree, Alexandre Torgue, linux-mmc, linux-kernel,
	srinivas.kandagatla, Ludovic Barre, Maxime Coquelin, linux-stm32,
	linux-arm-kernel
In-Reply-To: <20190813095951.26275-1-ludovic.Barre@st.com>

From: Ludovic Barre <ludovic.barre@st.com>

This patch adds busy_completion callback at mmci_host_ops
to allow to define a specific busy completion by variant.

The legacy code corresponding to busy completion used
by ux500 variants is moved to ux500_busy_complete function.

The busy_detect boolean property is replaced by
busy_complete callback definition.

Signed-off-by: Ludovic Barre <ludovic.barre@st.com>
---
 drivers/mmc/host/mmci.c | 140 +++++++++++++++++++++-------------------
 drivers/mmc/host/mmci.h |   3 +-
 2 files changed, 75 insertions(+), 68 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index c50586540765..9eac3f482119 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -44,6 +44,7 @@
 #define DRIVER_NAME "mmci-pl18x"
 
 static void mmci_variant_init(struct mmci_host *host);
+static void ux500_variant_init(struct mmci_host *host);
 static void ux500v2_variant_init(struct mmci_host *host);
 
 static unsigned int fmax = 515633;
@@ -175,7 +176,6 @@ static struct variant_data variant_ux500 = {
 	.f_max			= 100000000,
 	.signal_direction	= true,
 	.pwrreg_clkgate		= true,
-	.busy_detect		= true,
 	.busy_dpsm_flag		= MCI_DPSM_ST_BUSYMODE,
 	.busy_detect_flag	= MCI_ST_CARDBUSY,
 	.busy_detect_mask	= MCI_ST_BUSYENDMASK,
@@ -184,7 +184,7 @@ static struct variant_data variant_ux500 = {
 	.irq_pio_mask		= MCI_IRQ_PIO_MASK,
 	.start_err		= MCI_STARTBITERR,
 	.opendrain		= MCI_OD,
-	.init			= mmci_variant_init,
+	.init			= ux500_variant_init,
 };
 
 static struct variant_data variant_ux500v2 = {
@@ -208,7 +208,6 @@ static struct variant_data variant_ux500v2 = {
 	.f_max			= 100000000,
 	.signal_direction	= true,
 	.pwrreg_clkgate		= true,
-	.busy_detect		= true,
 	.busy_dpsm_flag		= MCI_DPSM_ST_BUSYMODE,
 	.busy_detect_flag	= MCI_ST_CARDBUSY,
 	.busy_detect_mask	= MCI_ST_BUSYENDMASK,
@@ -610,6 +609,67 @@ static u32 ux500v2_get_dctrl_cfg(struct mmci_host *host)
 	return MCI_DPSM_ENABLE | (host->data->blksz << 16);
 }
 
+static bool ux500_busy_complete(struct mmci_host *host, u32 status, u32 err_msk)
+{
+	void __iomem *base = host->base;
+
+	/*
+	 * Before unmasking for the busy end IRQ, confirm that the
+	 * command was sent successfully. To keep track of having a
+	 * command in-progress, waiting for busy signaling to end,
+	 * store the status in host->busy_status.
+	 *
+	 * Note that, the card may need a couple of clock cycles before
+	 * it starts signaling busy on DAT0, hence re-read the
+	 * MMCISTATUS register here, to allow the busy bit to be set.
+	 * Potentially we may even need to poll the register for a
+	 * while, to allow it to be set, but tests indicates that it
+	 * isn't needed.
+	 */
+	if (!host->busy_status && !(status & err_msk) &&
+	    (readl(base + MMCISTATUS) & host->variant->busy_detect_flag)) {
+		writel(readl(base + MMCIMASK0) |
+		       host->variant->busy_detect_mask,
+		       base + MMCIMASK0);
+
+		host->busy_status = status & (MCI_CMDSENT | MCI_CMDRESPEND);
+		return false;
+	}
+
+	/*
+	 * If there is a command in-progress that has been successfully
+	 * sent, then bail out if busy status is set and wait for the
+	 * busy end IRQ.
+	 *
+	 * Note that, the HW triggers an IRQ on both edges while
+	 * monitoring DAT0 for busy completion, but there is only one
+	 * status bit in MMCISTATUS for the busy state. Therefore
+	 * both the start and the end interrupts needs to be cleared,
+	 * one after the other. So, clear the busy start IRQ here.
+	 */
+	if (host->busy_status &&
+	    (status & host->variant->busy_detect_flag)) {
+		writel(host->variant->busy_detect_mask, base + MMCICLEAR);
+		return false;
+	}
+
+	/*
+	 * If there is a command in-progress that has been successfully
+	 * sent and the busy bit isn't set, it means we have received
+	 * the busy end IRQ. Clear and mask the IRQ, then continue to
+	 * process the command.
+	 */
+	if (host->busy_status) {
+		writel(host->variant->busy_detect_mask, base + MMCICLEAR);
+
+		writel(readl(base + MMCIMASK0) &
+		       ~host->variant->busy_detect_mask, base + MMCIMASK0);
+		host->busy_status = 0;
+	}
+
+	return true;
+}
+
 /*
  * All the DMA operation mode stuff goes inside this ifdef.
  * This assumes that you have a generic DMA device interface,
@@ -953,9 +1013,16 @@ void mmci_variant_init(struct mmci_host *host)
 	host->ops = &mmci_variant_ops;
 }
 
+void ux500_variant_init(struct mmci_host *host)
+{
+	host->ops = &mmci_variant_ops;
+	host->ops->busy_complete = ux500_busy_complete;
+}
+
 void ux500v2_variant_init(struct mmci_host *host)
 {
 	host->ops = &mmci_variant_ops;
+	host->ops->busy_complete = ux500_busy_complete;
 	host->ops->get_datactrl_cfg = ux500v2_get_dctrl_cfg;
 }
 
@@ -1239,68 +1306,9 @@ mmci_cmd_irq(struct mmci_host *host, struct mmc_command *cmd,
 		return;
 
 	/* Handle busy detection on DAT0 if the variant supports it. */
-	if (busy_resp && host->variant->busy_detect) {
-
-		/*
-		 * Before unmasking for the busy end IRQ, confirm that the
-		 * command was sent successfully. To keep track of having a
-		 * command in-progress, waiting for busy signaling to end,
-		 * store the status in host->busy_status.
-		 *
-		 * Note that, the card may need a couple of clock cycles before
-		 * it starts signaling busy on DAT0, hence re-read the
-		 * MMCISTATUS register here, to allow the busy bit to be set.
-		 * Potentially we may even need to poll the register for a
-		 * while, to allow it to be set, but tests indicates that it
-		 * isn't needed.
-		 */
-		if (!host->busy_status && !(status & err_msk) &&
-		    (readl(base + MMCISTATUS) & host->variant->busy_detect_flag)) {
-
-			writel(readl(base + MMCIMASK0) |
-			       host->variant->busy_detect_mask,
-			       base + MMCIMASK0);
-
-			host->busy_status =
-				status & (MCI_CMDSENT|MCI_CMDRESPEND);
-			return;
-		}
-
-		/*
-		 * If there is a command in-progress that has been successfully
-		 * sent, then bail out if busy status is set and wait for the
-		 * busy end IRQ.
-		 *
-		 * Note that, the HW triggers an IRQ on both edges while
-		 * monitoring DAT0 for busy completion, but there is only one
-		 * status bit in MMCISTATUS for the busy state. Therefore
-		 * both the start and the end interrupts needs to be cleared,
-		 * one after the other. So, clear the busy start IRQ here.
-		 */
-		if (host->busy_status &&
-		    (status & host->variant->busy_detect_flag)) {
-			writel(host->variant->busy_detect_mask,
-			       host->base + MMCICLEAR);
+	if (busy_resp && host->ops->busy_complete)
+		if (!host->ops->busy_complete(host, status, err_msk))
 			return;
-		}
-
-		/*
-		 * If there is a command in-progress that has been successfully
-		 * sent and the busy bit isn't set, it means we have received
-		 * the busy end IRQ. Clear and mask the IRQ, then continue to
-		 * process the command.
-		 */
-		if (host->busy_status) {
-
-			writel(host->variant->busy_detect_mask,
-			       host->base + MMCICLEAR);
-
-			writel(readl(base + MMCIMASK0) &
-			       ~host->variant->busy_detect_mask,
-			       base + MMCIMASK0);
-			host->busy_status = 0;
-		}
-	}
 
 	host->cmd = NULL;
 
@@ -1541,7 +1549,7 @@ static irqreturn_t mmci_irq(int irq, void *dev_id)
 		 * clear the corresponding IRQ.
 		 */
 		status &= readl(host->base + MMCIMASK0);
-		if (host->variant->busy_detect)
+		if (host->ops->busy_complete)
 			writel(status & ~host->variant->busy_detect_mask,
 			       host->base + MMCICLEAR);
 		else
@@ -1968,7 +1976,7 @@ static int mmci_probe(struct amba_device *dev,
 	/*
 	 * Enable busy detection.
 	 */
-	if (variant->busy_detect) {
+	if (host->ops->busy_complete) {
 		u32 max_busy_timeout = 0;
 
 		mmci_ops.card_busy = mmci_card_busy;
diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h
index d8b7f6774e8f..733f9a035b06 100644
--- a/drivers/mmc/host/mmci.h
+++ b/drivers/mmc/host/mmci.h
@@ -286,7 +286,6 @@ struct mmci_host;
  * @f_max: maximum clk frequency supported by the controller.
  * @signal_direction: input/out direction of bus signals can be indicated
  * @pwrreg_clkgate: MMCIPOWER register must be used to gate the clock
- * @busy_detect: true if the variant supports busy detection on DAT0.
  * @busy_timeout: true if the variant starts data timer when the DPSM
  *		  enter in Wait_R or Busy state.
  * @busy_dpsm_flag: bitmask enabling busy detection in the DPSM
@@ -334,7 +333,6 @@ struct variant_data {
 	u32			f_max;
 	u8			signal_direction:1;
 	u8			pwrreg_clkgate:1;
-	u8			busy_detect:1;
 	u8			busy_timeout:1;
 	u32			busy_dpsm_flag;
 	u32			busy_detect_flag;
@@ -369,6 +367,7 @@ struct mmci_host_ops {
 	void (*dma_error)(struct mmci_host *host);
 	void (*set_clkreg)(struct mmci_host *host, unsigned int desired);
 	void (*set_pwrreg)(struct mmci_host *host, unsigned int pwr);
+	bool (*busy_complete)(struct mmci_host *host, u32 status, u32 err_msk);
 };
 
 struct mmci_host {
-- 
2.17.1


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

^ permalink raw reply related

* [PATCH V5 1/3] mmc: mmci: add hardware busy timeout feature
From: Ludovic Barre @ 2019-08-13  9:59 UTC (permalink / raw)
  To: Ulf Hansson, Rob Herring
  Cc: devicetree, Alexandre Torgue, linux-mmc, linux-kernel,
	srinivas.kandagatla, Ludovic Barre, Maxime Coquelin, linux-stm32,
	linux-arm-kernel
In-Reply-To: <20190813095951.26275-1-ludovic.Barre@st.com>

From: Ludovic Barre <ludovic.barre@st.com>

In some variants, the data timer starts and decrements
when the DPSM enters in Wait_R or Busy state
(while data transfer or MMC_RSP_BUSY), and generates a
data timeout error if the counter reach 0.

-Define max_busy_timeout (in ms) according to clock.
-Set data timer register if the command has rsp_busy flag.
 If busy_timeout is not defined by framework, the busy
 length after Data Burst is defined as 1 second
 (refer: 4.6.2.2 Write of sd specification part1 v6-0).
-Add MCI_DATATIMEOUT error management in mmci_cmd_irq.

Signed-off-by: Ludovic Barre <ludovic.barre@st.com>
---
 drivers/mmc/host/mmci.c | 37 ++++++++++++++++++++++++++++++++-----
 drivers/mmc/host/mmci.h |  3 +++
 2 files changed, 35 insertions(+), 5 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index c37e70dbe250..c50586540765 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -1075,6 +1075,7 @@ static void
 mmci_start_command(struct mmci_host *host, struct mmc_command *cmd, u32 c)
 {
 	void __iomem *base = host->base;
+	unsigned long long clks = 0;
 
 	dev_dbg(mmc_dev(host->mmc), "op %02x arg %08x flags %08x\n",
 	    cmd->opcode, cmd->arg, cmd->flags);
@@ -1097,6 +1098,19 @@ mmci_start_command(struct mmci_host *host, struct mmc_command *cmd, u32 c)
 		else
 			c |= host->variant->cmdreg_srsp;
 	}
+
+	if (host->variant->busy_timeout && !host->mrq->data) {
+		if (cmd->flags & MMC_RSP_BUSY) {
+			if (!cmd->busy_timeout)
+				cmd->busy_timeout = 1000;
+
+			clks = (unsigned long long)cmd->busy_timeout;
+			clks *=	host->cclk;
+			do_div(clks, MSEC_PER_SEC);
+		}
+		writel_relaxed(clks, host->base + MMCIDATATIMER);
+	}
+
 	if (/*interrupt*/0)
 		c |= MCI_CPSM_INTERRUPT;
 
@@ -1203,6 +1217,7 @@ mmci_cmd_irq(struct mmci_host *host, struct mmc_command *cmd,
 {
 	void __iomem *base = host->base;
 	bool sbc, busy_resp;
+	u32 err_msk;
 
 	if (!cmd)
 		return;
@@ -1215,8 +1230,12 @@ mmci_cmd_irq(struct mmci_host *host, struct mmc_command *cmd,
 	 * handling. Note that we tag on any latent IRQs postponed
 	 * due to waiting for busy status.
 	 */
-	if (!((status|host->busy_status) &
-	      (MCI_CMDCRCFAIL|MCI_CMDTIMEOUT|MCI_CMDSENT|MCI_CMDRESPEND)))
+	err_msk = MCI_CMDCRCFAIL | MCI_CMDTIMEOUT;
+	if (host->variant->busy_timeout && busy_resp)
+		err_msk |= MCI_DATATIMEOUT;
+
+	if (!((status | host->busy_status) &
+	      (err_msk | MCI_CMDSENT | MCI_CMDRESPEND)))
 		return;
 
 	/* Handle busy detection on DAT0 if the variant supports it. */
@@ -1235,8 +1254,7 @@ mmci_cmd_irq(struct mmci_host *host, struct mmc_command *cmd,
 		 * while, to allow it to be set, but tests indicates that it
 		 * isn't needed.
 		 */
-		if (!host->busy_status &&
-		    !(status & (MCI_CMDCRCFAIL|MCI_CMDTIMEOUT)) &&
+		if (!host->busy_status && !(status & err_msk) &&
 		    (readl(base + MMCISTATUS) & host->variant->busy_detect_flag)) {
 
 			writel(readl(base + MMCIMASK0) |
@@ -1290,6 +1308,9 @@ mmci_cmd_irq(struct mmci_host *host, struct mmc_command *cmd,
 		cmd->error = -ETIMEDOUT;
 	} else if (status & MCI_CMDCRCFAIL && cmd->flags & MMC_RSP_CRC) {
 		cmd->error = -EILSEQ;
+	} else if (host->variant->busy_timeout && busy_resp &&
+		   status & MCI_DATATIMEOUT) {
+		cmd->error = -ETIMEDOUT;
 	} else {
 		cmd->resp[0] = readl(base + MMCIRESPONSE0);
 		cmd->resp[1] = readl(base + MMCIRESPONSE1);
@@ -1948,6 +1969,8 @@ static int mmci_probe(struct amba_device *dev,
 	 * Enable busy detection.
 	 */
 	if (variant->busy_detect) {
+		u32 max_busy_timeout = 0;
+
 		mmci_ops.card_busy = mmci_card_busy;
 		/*
 		 * Not all variants have a flag to enable busy detection
@@ -1957,7 +1980,11 @@ static int mmci_probe(struct amba_device *dev,
 			mmci_write_datactrlreg(host,
 					       host->variant->busy_dpsm_flag);
 		mmc->caps |= MMC_CAP_WAIT_WHILE_BUSY;
-		mmc->max_busy_timeout = 0;
+
+		if (variant->busy_timeout)
+			max_busy_timeout = ~0UL / (mmc->f_max / MSEC_PER_SEC);
+
+		mmc->max_busy_timeout = max_busy_timeout;
 	}
 
 	/* Prepare a CMD12 - needed to clear the DPSM on some variants. */
diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h
index 833236ecb31e..d8b7f6774e8f 100644
--- a/drivers/mmc/host/mmci.h
+++ b/drivers/mmc/host/mmci.h
@@ -287,6 +287,8 @@ struct mmci_host;
  * @signal_direction: input/out direction of bus signals can be indicated
  * @pwrreg_clkgate: MMCIPOWER register must be used to gate the clock
  * @busy_detect: true if the variant supports busy detection on DAT0.
+ * @busy_timeout: true if the variant starts data timer when the DPSM
+ *		  enter in Wait_R or Busy state.
  * @busy_dpsm_flag: bitmask enabling busy detection in the DPSM
  * @busy_detect_flag: bitmask identifying the bit in the MMCISTATUS register
  *		      indicating that the card is busy
@@ -333,6 +335,7 @@ struct variant_data {
 	u8			signal_direction:1;
 	u8			pwrreg_clkgate:1;
 	u8			busy_detect:1;
+	u8			busy_timeout:1;
 	u32			busy_dpsm_flag;
 	u32			busy_detect_flag;
 	u32			busy_detect_mask;
-- 
2.17.1


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

^ permalink raw reply related

* Re: [PATCH 1/2] arm64: Add initial support for E0PD
From: Suzuki K Poulose @ 2019-08-13  9:59 UTC (permalink / raw)
  To: broonie, catalin.marinas, will; +Cc: linux-arm-kernel
In-Reply-To: <20190812125738.17388-2-broonie@kernel.org>



On 12/08/2019 13:57, Mark Brown wrote:
> Kernel Page Table Isolation (KPTI) is used to mitigate some speculation
> based security issues by ensuring that the kernel is not mapped when
> userspace is running but this approach is expensive and is incompatible
> with SPE.  E0PD, introduced in the ARMv8.5 extensions, provides an
> alternative to this which ensures that accesses from userspace to the
> kernel's half of the memory map to always fault with constant time,
> preventing timing attacks without requiring constant unmapping and
> remapping or preventing legitimate accesses.
> 
> This initial patch does not yet integrate with KPTI, this will be dealt
> with in followup patches.  Ideally we could ensure that by default we
> don't use KPTI on CPUs where E0PD is present.
> 
> Signed-off-by: Mark Brown <broonie@kernel.org>
> ---
>   arch/arm64/Kconfig                     | 14 +++++++++++++
>   arch/arm64/include/asm/cpucaps.h       |  3 ++-
>   arch/arm64/include/asm/pgtable-hwdef.h |  2 ++
>   arch/arm64/include/asm/sysreg.h        |  1 +
>   arch/arm64/kernel/cpufeature.c         | 27 ++++++++++++++++++++++++++
>   5 files changed, 46 insertions(+), 1 deletion(-)
> 

Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>

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

^ permalink raw reply

* Re: [EXT] Re: [PATCHv5 1/2] PCI: layerscape: Add the bar_fixed_64bit property in EP driver.
From: Lorenzo Pieralisi @ 2019-08-13  9:51 UTC (permalink / raw)
  To: Xiaowei Bao
  Cc: linux-arm-kernel@lists.infradead.org, Roy Zang, Leonard Crestez,
	hayashi.kunihiko@socionext.com, andrew.smirnov@gmail.com,
	linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org,
	yue.wang@amlogic.com, Kishon Vijay Abraham I, M.h. Lian,
	dwmw@amazon.co.uk, jonnyc@amazon.com, bhelgaas@google.com,
	tpiepho@impinj.com, linuxppc-dev@lists.ozlabs.org, Mingkai Hu,
	l.stach@pengutronix.de
In-Reply-To: <AM5PR04MB32993CC1344DD660A298C7E1F5D20@AM5PR04MB3299.eurprd04.prod.outlook.com>

You should fix your email client set-up to avoid sticking an [EXT]
tag to your emails $SUBJECT.

On Tue, Aug 13, 2019 at 07:39:48AM +0000, Xiaowei Bao wrote:
> 
> 
> > -----Original Message-----
> > From: Kishon Vijay Abraham I <kishon@ti.com>
> > Sent: 2019年8月13日 15:30
> > To: Xiaowei Bao <xiaowei.bao@nxp.com>; lorenzo.pieralisi@arm.com;
> > bhelgaas@google.com; M.h. Lian <minghuan.lian@nxp.com>; Mingkai Hu
> > <mingkai.hu@nxp.com>; Roy Zang <roy.zang@nxp.com>;
> > l.stach@pengutronix.de; tpiepho@impinj.com; Leonard Crestez
> > <leonard.crestez@nxp.com>; andrew.smirnov@gmail.com;
> > yue.wang@amlogic.com; hayashi.kunihiko@socionext.com;
> > dwmw@amazon.co.uk; jonnyc@amazon.com; linux-pci@vger.kernel.org;
> > linux-kernel@vger.kernel.org; linuxppc-dev@lists.ozlabs.org;
> > linux-arm-kernel@lists.infradead.org
> > Subject: [EXT] Re: [PATCHv5 1/2] PCI: layerscape: Add the bar_fixed_64bit
> > property in EP driver.
> > 
> > Caution: EXT Email

See above, this "Caution" stuff should disappear.

Also, quoting the email header is useless, please configure your email
client to remove it.

Thanks,
Lorenzo

> > On 13/08/19 11:58 AM, Xiaowei Bao wrote:
> > > The PCIe controller of layerscape just have 4 BARs, BAR0 and BAR1 is
> > > 32bit, BAR2 and BAR4 is 64bit, this is determined by hardware, so set
> > > the bar_fixed_64bit with 0x14.
> > >
> > > Signed-off-by: Xiaowei Bao <xiaowei.bao@nxp.com>
> > 
> > Acked-by: Kishon Vijay Abraham I <kishon@ti.com>
> > > ---
> > > v2:
> > >  - Replace value 0x14 with a macro.
> > > v3:
> > >  - No change.
> > > v4:
> > >  - send the patch again with '--to'.
> > > v5:
> > >  - fix the commit message.
> > >
> > >  drivers/pci/controller/dwc/pci-layerscape-ep.c | 1 +
> > >  1 file changed, 1 insertion(+)
> > >
> > > diff --git a/drivers/pci/controller/dwc/pci-layerscape-ep.c
> > > b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> > > index be61d96..ca9aa45 100644
> > > --- a/drivers/pci/controller/dwc/pci-layerscape-ep.c
> > > +++ b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> > > @@ -44,6 +44,7 @@ static const struct pci_epc_features
> > ls_pcie_epc_features = {
> > >       .linkup_notifier = false,
> > >       .msi_capable = true,
> > >       .msix_capable = false,
> > > +     .bar_fixed_64bit = (1 << BAR_2) | (1 << BAR_4),
> > >  };
> > >
> > >  static const struct pci_epc_features*
> I check other platforms, it is 'static const struct pci_epc_features', I can get the correct 
> Value use this define way in pci-epf-test.c file.
> > >

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

^ permalink raw reply


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