Devicetree
 help / color / mirror / Atom feed
* [PATCH v4 2/2] power: supply: ltc2941-battery-gauge: Add OF device ID table
From: Javier Martinez Canillas @ 2017-03-29 16:00 UTC (permalink / raw)
  To: linux-kernel
  Cc: devicetree, Mark Rutland, Rob Herring, Javier Martinez Canillas,
	Sebastian Reichel, linux-pm
In-Reply-To: <20170329160057.8298-1-javier@osg.samsung.com>

The driver doesn't have a struct of_device_id table but supported devices
are registered via Device Trees. This is working on the assumption that a
I2C device registered via OF will always match a legacy I2C device ID and
that the MODALIAS reported will always be of the form i2c:<device>.

But this could change in the future so the correct approach is to have an
OF device ID table if the devices are registered via OF.

Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>

---

Changes in v4:
- Add a vendor prefix to OF device compatible strings (Sebastian Reichel).

Changes in v3:
- Don't use of_match_ptr() to avoid build warning when CONFIG_OF is disabled.

Changes in v2:
- Fix build warning reported by kbuild test robot.
- Fix wrong compatible strings due a copy & paste error.

 drivers/power/supply/ltc2941-battery-gauge.c | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/drivers/power/supply/ltc2941-battery-gauge.c b/drivers/power/supply/ltc2941-battery-gauge.c
index 4adf2ba021ce..7efb908f4451 100644
--- a/drivers/power/supply/ltc2941-battery-gauge.c
+++ b/drivers/power/supply/ltc2941-battery-gauge.c
@@ -9,6 +9,7 @@
  */
 #include <linux/kernel.h>
 #include <linux/module.h>
+#include <linux/of_device.h>
 #include <linux/types.h>
 #include <linux/errno.h>
 #include <linux/swab.h>
@@ -61,7 +62,7 @@ struct ltc294x_info {
 	struct power_supply *supply;	/* Supply pointer */
 	struct power_supply_desc supply_desc;	/* Supply description */
 	struct delayed_work work;	/* Work scheduler */
-	int num_regs;	/* Number of registers (chip type) */
+	unsigned long num_regs;	/* Number of registers (chip type) */
 	int charge;	/* Last charge register content */
 	int r_sense;	/* mOhm */
 	int Qlsb;	/* nAh */
@@ -387,7 +388,7 @@ static int ltc294x_i2c_probe(struct i2c_client *client,
 
 	np = of_node_get(client->dev.of_node);
 
-	info->num_regs = id->driver_data;
+	info->num_regs = (unsigned long)of_device_get_match_data(&client->dev);
 	info->supply_desc.name = np->name;
 
 	/* r_sense can be negative, when sense+ is connected to the battery
@@ -497,9 +498,23 @@ static const struct i2c_device_id ltc294x_i2c_id[] = {
 };
 MODULE_DEVICE_TABLE(i2c, ltc294x_i2c_id);
 
+static const struct of_device_id ltc294x_i2c_of_match[] = {
+	{
+		.compatible = "lltc,ltc2941",
+		.data = (void *)LTC2941_NUM_REGS
+	},
+	{
+		.compatible = "lltc,ltc2943",
+		.data = (void *)LTC2943_NUM_REGS
+	},
+	{ },
+};
+MODULE_DEVICE_TABLE(of, ltc294x_i2c_of_match);
+
 static struct i2c_driver ltc294x_driver = {
 	.driver = {
 		.name	= "LTC2941",
+		.of_match_table = ltc294x_i2c_of_match,
 		.pm	= LTC294X_PM_OPS,
 	},
 	.probe		= ltc294x_i2c_probe,
-- 
2.9.3

^ permalink raw reply related

* RE: [PATCH 2/2] soc/imx: add workaround for i.MX6QP to the GPC PD driver
From: A.S. Dong @ 2017-03-29 16:08 UTC (permalink / raw)
  To: Lucas Stach, Shawn Guo
  Cc: Fabio Estevam, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	kernel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org,
	patchwork-lst-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org
In-Reply-To: <20170323144418.30977-3-l.stach-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>

Hi Lucas,

> -----Original Message-----
> From: Lucas Stach [mailto:l.stach-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org]
> Sent: Thursday, March 23, 2017 10:44 PM
> To: Shawn Guo
> Cc: Fabio Estevam; A.S. Dong; devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; linux-arm-
> kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org; kernel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org; patchwork-
> lst-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org
> Subject: [PATCH 2/2] soc/imx: add workaround for i.MX6QP to the GPC PD
> driver
> 
> On i.MX6QP, due to hardware erratum ERR009619, the PRE clocks may be
> stalled during the power up sequencing of the PU power domain. As this may
> lead to a complete loss of display output, the recommended workaround is
> to keep the PU domain enabled during normal system operation.
> 
> Implement this by rejecting the domain power down request on the affected
> SoC.
> 
> Signed-off-by: Lucas Stach <l.stach-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> ---
>  drivers/soc/imx/gpc.c | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/drivers/soc/imx/gpc.c b/drivers/soc/imx/gpc.c index
> 4294287e5f6c..599e1e46f694 100644
> --- a/drivers/soc/imx/gpc.c
> +++ b/drivers/soc/imx/gpc.c
> @@ -45,6 +45,7 @@ struct imx_pm_domain {
>  	unsigned int reg_offs;
>  	signed char cntr_pdn_bit;
>  	unsigned int ipg_rate_mhz;
> +	bool allow_dynamic_pd;
>  };
> 
>  static inline struct imx_pm_domain *
> @@ -59,6 +60,9 @@ static int imx6_pm_domain_power_off(struct
> generic_pm_domain *genpd)
>  	int iso, iso2sw;
>  	u32 val;
> 
> +	if (!pd->allow_dynamic_pd)
> +		return -EBUSY;
> +
>  	/* Read ISO and ISO2SW power down delays */
>  	regmap_read(pd->regmap, pd->reg_offs + GPC_PGC_PUPSCR_OFFS, &val);
>  	iso = val & 0x3f;
> @@ -255,6 +259,7 @@ static struct imx_pm_domain imx_gpc_domains[] = {
>  		},
>  		.reg_offs = 0x260,
>  		.cntr_pdn_bit = 0,
> +		.allow_dynamic_pd = true,
>  	}, {
>  		.base = {
>  			.name = "DISPLAY",
> @@ -263,23 +268,33 @@ static struct imx_pm_domain imx_gpc_domains[] = {
>  		},
>  		.reg_offs = 0x240,
>  		.cntr_pdn_bit = 4,
> +		.allow_dynamic_pd = true,

Just a Nitpick, besides shawn's comment in another mail,
if we use a domain flag like IMX_PD_ALWAYS_ON set by SoC errata flag,
then probably we can save all the default allow_dynamic_pd lines.
This also release the life when adding new domains.

Otherwise, the two patch seems good to me.

Regards
Dong Aisheng

>  	}
>  };
> 
>  struct imx_gpc_dt_data {
>  	int num_domains;
> +	bool err009619_present;
>  };
> 
>  static const struct imx_gpc_dt_data imx6q_dt_data = {
>  	.num_domains = 2,
> +	.err009619_present = false,
> +};
> +
> +static const struct imx_gpc_dt_data imx6qp_dt_data = {
> +	.num_domains = 2,
> +	.err009619_present = true,
>  };
> 
>  static const struct imx_gpc_dt_data imx6sl_dt_data = {
>  	.num_domains = 3,
> +	.err009619_present = false,
>  };
> 
>  static const struct of_device_id imx_gpc_dt_ids[] = {
>  	{ .compatible = "fsl,imx6q-gpc", .data = &imx6q_dt_data },
> +	{ .compatible = "fsl,imx6qp-gpc", .data = &imx6qp_dt_data },
>  	{ .compatible = "fsl,imx6sl-gpc", .data = &imx6sl_dt_data },
>  	{ }
>  };
> @@ -377,6 +392,10 @@ static int imx_gpc_probe(struct platform_device *pdev)
>  		return ret;
>  	}
> 
> +	/* Disable PU power down in normal operation if ERR009619 is present
> */
> +	if (of_id_data->err009619_present)
> +		imx_gpc_domains[1].allow_dynamic_pd = false;
> +
>  	if (!pgc_node) {
>  		ret = imx_gpc_old_dt_init(&pdev->dev, regmap,
>  					  of_id_data->num_domains);
> --
> 2.11.0

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH] ARM: dts: keystone-k2l: fix broken Ethernet due to disabled OSR
From: santosh.shilimkar-QHcLZuEGTsvQT0dZR+AlfA @ 2017-03-29 16:13 UTC (permalink / raw)
  To: arm-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org
  Cc: Sekhar Nori, Santosh Shilimkar, Rob Herring, Mark Rutland,
	Russell King, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Tero Kristo,
	Murali Karicheri
In-Reply-To: <59ba5ef37f17ff1c10d9d52ca06ef341b2d476eb.1490782833.git.nsekhar-l0cyMroinI0@public.gmane.org>

Hi Arnd, Olof,

On 3/29/17 5:32 AM, Sekhar Nori wrote:
> From: Murali Karicheri <m-karicheri2-l0cyMroinI0@public.gmane.org>
>
> Ethernet networking on K2L has been broken since v4.11-rc1. This was
> caused by commit 32a34441a9bd ("ARM: keystone: dts: fix netcp clocks
> and add names"). This commit inadvertently moves on-chip static RAM
> clock to the end of list of clocks provided for netcp. Since keystone
> PM domain support does not have a list of recognized con_ids, only the
> first clock in the list comes under runtime PM management. This means
> the OSR (On-chip Static RAM) clock remains disabled and that broke
> networking on K2L.
>
> The OSR is used by QMSS on K2L as an external linking RAM. However this
> is a standalone RAM that can be used for non-QMSS usage (as well as from
> DSP side). So add a SRAM device node for the same and add the OSR clock
> to the node.
>
> Remove the now redundant OSR clock node from netcp.
>
> To manage all clocks defined for netCP's use by runtime PM needs keystone
> generic power domain (genpd) driver support which is under works.
> Meanwhile, this patch restores K2L networking and is correct irrespective
> of any future genpd work since OSR is an independent module and not part
> of NetCP anyway.
>
> Signed-off-by: Murali Karicheri <m-karicheri2-l0cyMroinI0@public.gmane.org>
> Acked-by: Tero Kristo <t-kristo-l0cyMroinI0@public.gmane.org>
> [nsekhar-l0cyMroinI0@public.gmane.org: commit message updates, port to latest mainline]
> Signed-off-by: Sekhar Nori <nsekhar-l0cyMroinI0@public.gmane.org>
> ---
Acked-by: Santosh Shilimkar <ssantosh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

Can you please pick this up for rcx fixes ?

Regards,
Santosh

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 2/2] soc/imx: add workaround for i.MX6QP to the GPC PD driver
From: Lucas Stach @ 2017-03-29 16:13 UTC (permalink / raw)
  To: A.S. Dong
  Cc: Shawn Guo, Fabio Estevam,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	kernel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org,
	patchwork-lst-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org
In-Reply-To: <DB5PR04MB1431A7CC91F55ECFE5BE31FB80350-rqLcZCCNzVwrYIBjo4yUUM9NdZoXdze2vxpqHgZTriW3zl9H0oFU5g@public.gmane.org>

Am Mittwoch, den 29.03.2017, 16:08 +0000 schrieb A.S. Dong:
> Hi Lucas,
> 
> > -----Original Message-----
> > From: Lucas Stach [mailto:l.stach-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org]
> > Sent: Thursday, March 23, 2017 10:44 PM
> > To: Shawn Guo
> > Cc: Fabio Estevam; A.S. Dong; devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; linux-arm-
> > kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org; kernel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org; patchwork-
> > lst-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org
> > Subject: [PATCH 2/2] soc/imx: add workaround for i.MX6QP to the GPC PD
> > driver
> > 
> > On i.MX6QP, due to hardware erratum ERR009619, the PRE clocks may be
> > stalled during the power up sequencing of the PU power domain. As this may
> > lead to a complete loss of display output, the recommended workaround is
> > to keep the PU domain enabled during normal system operation.
> > 
> > Implement this by rejecting the domain power down request on the affected
> > SoC.
> > 
> > Signed-off-by: Lucas Stach <l.stach-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> > ---
> >  drivers/soc/imx/gpc.c | 19 +++++++++++++++++++
> >  1 file changed, 19 insertions(+)
> > 
> > diff --git a/drivers/soc/imx/gpc.c b/drivers/soc/imx/gpc.c index
> > 4294287e5f6c..599e1e46f694 100644
> > --- a/drivers/soc/imx/gpc.c
> > +++ b/drivers/soc/imx/gpc.c
> > @@ -45,6 +45,7 @@ struct imx_pm_domain {
> >  	unsigned int reg_offs;
> >  	signed char cntr_pdn_bit;
> >  	unsigned int ipg_rate_mhz;
> > +	bool allow_dynamic_pd;
> >  };
> > 
> >  static inline struct imx_pm_domain *
> > @@ -59,6 +60,9 @@ static int imx6_pm_domain_power_off(struct
> > generic_pm_domain *genpd)
> >  	int iso, iso2sw;
> >  	u32 val;
> > 
> > +	if (!pd->allow_dynamic_pd)
> > +		return -EBUSY;
> > +
> >  	/* Read ISO and ISO2SW power down delays */
> >  	regmap_read(pd->regmap, pd->reg_offs + GPC_PGC_PUPSCR_OFFS, &val);
> >  	iso = val & 0x3f;
> > @@ -255,6 +259,7 @@ static struct imx_pm_domain imx_gpc_domains[] = {
> >  		},
> >  		.reg_offs = 0x260,
> >  		.cntr_pdn_bit = 0,
> > +		.allow_dynamic_pd = true,
> >  	}, {
> >  		.base = {
> >  			.name = "DISPLAY",
> > @@ -263,23 +268,33 @@ static struct imx_pm_domain imx_gpc_domains[] = {
> >  		},
> >  		.reg_offs = 0x240,
> >  		.cntr_pdn_bit = 4,
> > +		.allow_dynamic_pd = true,
> 
> Just a Nitpick, besides shawn's comment in another mail,
> if we use a domain flag like IMX_PD_ALWAYS_ON set by SoC errata flag,
> then probably we can save all the default allow_dynamic_pd lines.
> This also release the life when adding new domains.

If other things like that show up we can certainly switch to a flags
field. In the meantime I like that the current style is more explicit
about it.

Regards,
Lucas


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 1/1] of: Move OF property and graph API from base.c to property.c
From: Frank Rowand @ 2017-03-29 16:28 UTC (permalink / raw)
  To: Sakari Ailus, linux-acpi, devicetree, robh
  Cc: sudeep.holla, lorenzo.pieralisi, mika.westerberg, rafael,
	mark.rutland, broonie, ahs3
In-Reply-To: <fb9cdd4e-e837-f925-da2b-9e4d25602e1e@linux.intel.com>

On 03/29/17 02:14, Sakari Ailus wrote:
> Hi Frank,
> 
> Thank you for the review.
> 
> Frank Rowand wrote:
>> Hi Sakari,
>>
>> On 03/24/17 03:59, Sakari Ailus wrote:
>>> base.c contains both core OF functions and increasingly other
>>> functionality such as accessing properties and graphs, including
>>> convenience functions. In the near future this would also include OF
>>> specific implementation of the fwnode property and graph APIs. Prepare for
>>> this by moving the functionality to property.c.
>>
>> For future me, for when I look back and read this commit, please add something
>> to the effect of:
>>
>>   Create driver/of/property.c to contain procedures for accessing and
>>   interpreting device tree properties.  The procedures are moved from
>>   drivers/of/base.c, with no changes other than copying only the
>>   includes required by the moved procedures.
> 
> How about this as the commit message combining the two and dropping the last sentence from the original description:
> 
> base.c contains both core OF functions and increasingly other
> functionality such as accessing properties and graphs, including
> convenience functions. In the near future this would also include OF
> specific implementation of the fwnode property and graph APIs.
> 
> Create driver/of/property.c to contain procedures for accessing and
> interpreting device tree properties. The procedures are moved from
> drivers/of/base.c, with no changes other than copying only the
> includes required by the moved procedures.

Sounds good (and same for your below comments).

-Frank

> 
> ...
> 
>>> diff --git a/drivers/of/property.c b/drivers/of/property.c
>>> new file mode 100644
>>> index 0000000..afcf00a
>>> --- /dev/null
>>> +++ b/drivers/of/property.c
>>> @@ -0,0 +1,727 @@
>>> +/*
>>> + * drivers/of/property.c - Devicetree properties and graphs.
>>
>>     * drivers/of/property.c - Procedures for accessing and interpreting
>>     * Devicetree properties and graphs.
>>     *
>>     * Initially created by copying procedures from drivers/of/base.c
>>
>> The reference to base.c provides a documentation trail to all of the
>> earlier git commits that created and modified the procedures.
> 
> I'll add that.
> 
>>
>> It would also be useful to say what procedures do and do not belong
>> in this file.  My understanding is that the intent is for
>> drivers/of/property.c and drivers/acpi/property.c to provide the
>> respective functionality that is used by drivers/base/property.c
>> and that drivers also may directly access the procedures in
>> drivers/of/property.c.
> 
> I'll add something along those lines. The fwnode op struct and the operations in that will be in this file as well.
> 


^ permalink raw reply

* Re: [PATCH 2/2] mtd: spi-nor: add driver for STM32 quad spi flash controller
From: Ludovic BARRE @ 2017-03-29 16:38 UTC (permalink / raw)
  To: Marek Vasut, Cyrille Pitchen
  Cc: David Woodhouse, Brian Norris, Boris Brezillon,
	Richard Weinberger, Alexandre Torgue, Rob Herring,
	linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <0e7da44f-c41b-de15-62c3-7509e556f623-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>



On 03/29/2017 03:57 PM, Marek Vasut wrote:
> On 03/29/2017 03:35 PM, Ludovic BARRE wrote:
>
> [...]
>
>>>>>> +    writel_relaxed(CR_PRESC(presc) | CR_FTHRES(3) | CR_TCEN |
>>>>>> CR_SSHIFT
>>>>>> +               | CR_EN, qspi->io_base + QUADSPI_CR);
>>>>>> +
>>>>>> +    /* a minimum fsize must be set to sent the command id */
>>>>>> +    flash->fsize = 25;
>>>>> I don't understand why this is needed and the comment doesn't make
>>>>> sense. Please fix.
>>>> fsize field defines the size of external memory.
>>> What external memory ? Unclear
>> oops, fsize field defined the size of "flash memory" in stm32 qspi
>> controller.
> Errr, now I am totally lost :) Is that some internal SPI NOR ? Shouldn't
> the size be coming from DT or something ?
>
>> Number of bytes in Flash memory = 2 ^[FSIZE+1].
>> To sent a nor cmd this field must be set (hardware issue),
>> but before "spi_nor_scan" the size of flash nor is not know.
>> So I set a temporary value (workaround).
> Is it needed before the scan ?
yes it's needed before scan (fix a "stm32 qspi controller" issue)

sorry, I try to reformulate:

The nor flash (external component like micron n25q128a13
or spansion s25fl512s ...) is connected to stm32 by classic
spi-nor interface cs, clock and 1/2/4 IO lines.

the stm32 microprocessor has a dedicated controller to
manage spi-nor interface, it's stm32 qspi.

In stm32 qspi controller there is a register with fsize field
which define the size of nor flash (n25q128a13 or s25fl512s...).

fsize can't be null, else the stm32 qspi controller doesn't send
spi-nor command. it's "stm32 qspi controller" issue.

Before the "spi_nor_scan" the size of nor flash (n25q128a13
or s25fl512s...) is not know. So we set a temporary value just
to discover the nor flash with "spi_nor_scan". After we can
set the right value (mtd->size) in fsize.
>> After "spi_nor_scan" the fsize is overwritten by the right value
>> flash->fsize = __fls(mtd->size) - 1;
>>>> Normaly, this field is used only for memory map mode,
>>>> but in fact is check in indirect mode.
>>>> So while flash scan "spi_nor_scan":
>>>> -I can't let 0.
>>>> -I not know yet the size of flash.
>>>> So I fix a temporary value
>>>>
>>>> I will update my comment
>>> Please do, also please consider that I'm reading the comment and I
>>> barely have any clue about this hardware , so make sure I can
>>> understand it.
>>>
>>>>>> +    ret = spi_nor_scan(&flash->nor, NULL, flash_read);
>>>>>> +    if (ret) {
>>>>>> +        dev_err(qspi->dev, "device scan failed\n");
>>>>>> +        return ret;
>>>>>> +    }
>>>>>> +
>>>>>> +    flash->fsize = __fls(mtd->size) - 1;
>>>>>> +
>>>>>> +    writel_relaxed(DCR_CSHT(1), qspi->io_base + QUADSPI_DCR);
>>>>>> +
>>>>>> +    ret = mtd_device_register(mtd, NULL, 0);
>>>>>> +    if (ret) {
>>>>>> +        dev_err(qspi->dev, "mtd device parse failed\n");
>>>>>> +        return ret;
>>>>>> +    }
>>>>>> +
>>>>>> +    dev_dbg(qspi->dev, "read mm:%s cs:%d bus:%d\n",
>>>>>> +        qspi->read_mode == CCR_FMODE_MM ? "yes" : "no", cs_num,
>>>>>> width);
>>>>>> +
>>>>>> +    return 0;
>>>>>> +}
>>>>> [...]
>>>>>
>

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH] Add hardware PTP support.
From: David Miller @ 2017-03-29 16:39 UTC (permalink / raw)
  To: rafalo-vna1KIf7WgpBDgjK7y7TUQ
  Cc: nicolas.ferre-AIFe0yeh4nAAvxtiuMwx3w,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	harinikatakamlinux-Re5JQEeQqe8AvxtiuMwx3w,
	harini.katakam-gjFFaj9aHVfQT0dZR+AlfA,
	richardcochran-Re5JQEeQqe8AvxtiuMwx3w,
	Andrei.Pistirica-UWL1GkI3JZL3oGB3hsPCZA
In-Reply-To: <1490796051-28527-1-git-send-email-rafalo-vna1KIf7WgpBDgjK7y7TUQ@public.gmane.org>

From: Rafal Ozieblo <rafalo-vna1KIf7WgpBDgjK7y7TUQ@public.gmane.org>
Date: Wed, 29 Mar 2017 15:00:51 +0100

> This patch is based on original Harini's patch and Andrei's patch,
> implemented in aseparate file to ease the review/maintanance
> and integration with other platforms.
> 
> In case that macb is compiled as a module, it has been renamed to
> cadence-macb.ko to avoid naming confusion in Makefile.
> 
> This driver does support GEM-GXL:
> - Enable HW time stamp
> - Register ptp clock framework
> - Initialize PTP related registers
> - Updated dma buffer descriptor read/write mechanism
> - HW time stamp on the PTP Ethernet packets are received using the
>   SO_TIMESTAMPING API. Where timers are obtained from the dma buffer
>   descriptors
> - Added tsu_clk to device tree
> 
> Note: Patch on net-next, on March 15th.
> 
> Signed-off-by: Rafal Ozieblo <rafalo-vna1KIf7WgpBDgjK7y7TUQ@public.gmane.org>

Your subject should provide a proper subsystem prefix such as
"cadence: " after "[PATCH] ".

Also, March 15th is nearly half a month ago, you should generate
patches against the current tree.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 0/2] mtd: spi-nor: add stm32 qspi driver
From: Cyrille Pitchen @ 2017-03-29 16:51 UTC (permalink / raw)
  To: Ludovic Barre, Cyrille Pitchen, Marek Vasut
  Cc: Boris Brezillon, Alexandre Torgue,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Richard Weinberger,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Rob Herring,
	linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Brian Norris,
	David Woodhouse
In-Reply-To: <1490619296-8168-1-git-send-email-ludovic.Barre-qxv4g6HH51o@public.gmane.org>

Hi Ludovic,

Le 27/03/2017 à 14:54, Ludovic Barre a écrit :
> From: Ludovic Barre <ludovic.barre-qxv4g6HH51o@public.gmane.org>
> 
> This patch set adds a SPI-NOR driver for stm32 QSPI controller.
> It is a specialized SPI interface for serial Flash devices.
> It supports 1 or 2 Flash device with single, dual and quad SPI Flash memories.
> 
> It can operate in any of the following modes:
> -indirect mode: all the operations are performed using the quadspi
>  registers
> -read memory-mapped mode: the external Flash memory is mapped to the
>  microcontroller address space and is seen by the system as if it was
>  an internal memory
> 
> Ludovic Barre (2):
>   dt-bindings: Document the STM32 QSPI bindings
>   mtd: spi-nor: add driver for STM32 quad spi flash controller
> 
>  .../devicetree/bindings/mtd/stm32-quadspi.txt      |  45 ++
>  drivers/mtd/spi-nor/Kconfig                        |   7 +
>  drivers/mtd/spi-nor/Makefile                       |   1 +
>  drivers/mtd/spi-nor/stm32-quadspi.c                | 679 +++++++++++++++++++++
>  4 files changed, 732 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mtd/stm32-quadspi.txt
>  create mode 100644 drivers/mtd/spi-nor/stm32-quadspi.c
> 

Just a small word to warn you that I'm likely to ask you to rebase this
series onto the patch "mtd: spi-nor: introduce more SPI protocols and
the Dual Transfer Mode". Indeed, I need to synchronize with Marek first
but I plan to merge this patch within few days.


Best regards,

Cyrille
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v5 6/9] coresight: add support for CPU debug module
From: Mathieu Poirier @ 2017-03-29 16:55 UTC (permalink / raw)
  To: Leo Yan
  Cc: Jonathan Corbet, Rob Herring, Mark Rutland, Wei Xu,
	Catalin Marinas, Will Deacon, Andy Gross, David Brown,
	Michael Turquette, Stephen Boyd, Guodong Xu, John Stultz,
	linux-doc, linux-kernel, devicetree, linux-arm-kernel,
	linux-arm-msm, linux-soc, linux-clk, mike.leach, Suzuki.Poulose,
	sudeep.holla
In-Reply-To: <20170329015423.GA5035@leoy-linaro>

On Wed, Mar 29, 2017 at 09:54:23AM +0800, Leo Yan wrote:
> Hi Mathieu,
> 
> On Tue, Mar 28, 2017 at 10:50:10AM -0600, Mathieu Poirier wrote:
> > On Sun, Mar 26, 2017 at 02:23:14AM +0800, Leo Yan wrote:
> 
> [...]
> 
> > > +static void debug_force_cpu_powered_up(struct debug_drvdata *drvdata)
> > > +{
> > > +	int timeout = DEBUG_WAIT_TIMEOUT;
> > > +
> > > +	drvdata->edprsr = readl_relaxed(drvdata->base + EDPRSR);
> > > +
> > > +	CS_UNLOCK(drvdata->base);
> > > +
> > > +	/* Bail out if CPU is powered up yet */
> > > +	if (drvdata->edprsr & EDPRSR_PU)
> > > +		goto out_powered_up;
> > > +
> > > +	/*
> > > +	 * Send request to power management controller and assert
> > > +	 * DBGPWRUPREQ signal; if power management controller has
> > > +	 * sane implementation, it should enable CPU power domain
> > > +	 * in case CPU is in low power state.
> > > +	 */
> > > +	drvdata->edprsr = readl(drvdata->base + EDPRCR);
> > > +	drvdata->edprsr |= EDPRCR_COREPURQ;
> > > +	writel(drvdata->edprsr, drvdata->base + EDPRCR);
> > 
> > Here ->edprsr is used but EDPRCR is accessed.  Is this intentional or a
> > copy/paste error?  The same is true for accesses in the out_powered_up section.
> 
> Thanks for pointing out. This is a typo error and will fix.
> 
> > > +
> > > +	/* Wait for CPU to be powered up (timeout~=32ms) */
> > > +	while (timeout--) {
> > > +		drvdata->edprsr = readl_relaxed(drvdata->base + EDPRSR);
> > > +		if (drvdata->edprsr & EDPRSR_PU)
> > > +			break;
> > > +
> > > +		usleep_range(1000, 2000);
> > > +	}
> > 
> > See if function readx_poll_timeout() can be used.
> 
> Will use it.
> 
> > > +
> > > +	/*
> > > +	 * Unfortunately the CPU cannot be powered up, so return
> > > +	 * back and later has no permission to access other
> > > +	 * registers. For this case, should set 'idle_constraint'
> > > +	 * to ensure CPU power domain is enabled!
> > > +	 */
> > > +	if (!(drvdata->edprsr & EDPRSR_PU)) {
> > > +		pr_err("%s: power up request for CPU%d failed\n",
> > > +			__func__, drvdata->cpu);
> > > +		goto out;
> > > +	}
> > > +
> > > +out_powered_up:
> > > +	debug_os_unlock(drvdata);
> > > +
> > > +	/*
> > > +	 * At this point the CPU is powered up, so set the no powerdown
> > > +	 * request bit so we don't lose power and emulate power down.
> > > +	 */
> > > +	drvdata->edprsr = readl(drvdata->base + EDPRCR);
> > > +	drvdata->edprsr |= EDPRCR_COREPURQ | EDPRCR_CORENPDRQ;
> > 
> > If we are here the core is already up.  Shouldn't we need to set
> > EDPRCR_CORENPDRQ only?
> 
> Yeah. Will fix.
> 
> > > +	writel(drvdata->edprsr, drvdata->base + EDPRCR);
> > 
> > This section is a little racy - between the time the PU bit has been
> > checked and the time COREPDRQ has been flipped, the state of PU may have
> > changed.  You can probably get around this by checking edprsr.PU rigth here.  If
> > it is not set you go through the process again.  Note that doing this will
> > probably force a refactoring of the whole function.  
> 
> Agree. Will handle this.
> 
> [...]
> 
> > > +static ssize_t debug_func_knob_write(struct file *f,
> > > +		const char __user *buf, size_t count, loff_t *ppos)
> > > +{
> > > e	u8 on;
> > > +	int ret;
> > > +
> > > +	ret = kstrtou8_from_user(buf, count, 2, &on);
> > > +	if (ret)
> > > +		return ret;
> > > +
> > > +	mutex_lock(&debug_lock);
> > > +
> > > +	if (!on ^ debug_enable)
> > > +		goto out;
> > 
> > I had to read this condition too many times - please refactor.
> 
> Will do it.
> 
> > > +
> > > +	if (on) {
> > > +		ret = debug_enable_func();
> > > +		if (ret) {
> > > +			pr_err("%s: unable to disable debug function: %d\n",
> > > +			       __func__, ret);
> > 
> > Based on the semantic this is the wrong error message.
> 
> Yeah. Will fix.
> 
> > > +			goto err;
> > > +		}
> > > +	} else
> > > +		debug_disable_func();
> > 
> > Did checkpatch.pl complain about extra curly braces?  If not please add them.
> 
> checkpatch.pl doesn't report for this. Will add.
> 
> > > +
> > > +	debug_enable = on;
> > 
> > Here we can't set debug_enable if we just called debug_disable_func().  Maybe
> > I'm missing something.  If that's the case a comment in the code would be worth
> > it.
> 
> After called debug_disable_func(), debug_enable is set to 0 (on = 0).
> 
> > > +
> > > +out:
> > > +	ret = count;
> > > +err:
> > > +	mutex_unlock(&debug_lock);
> > > +	return ret;
> > > +}
> > > +
> > > +static ssize_t debug_func_knob_read(struct file *f,
> > > +		char __user *ubuf, size_t count, loff_t *ppos)
> > > +{
> > > +	char val[] = { '0' + debug_enable, '\n' };
> > > +
> > > +	return simple_read_from_buffer(ubuf, count, ppos, val, sizeof(val));
> > 
> > Use the debug_lock to avoid race conditions.
> 
> Will do it.
> 
> > > +}
> > > +
> > > +static ssize_t debug_idle_constraint_write(struct file *f,
> > > +		const char __user *buf, size_t count, loff_t *ppos)
> > > +{
> > > +	int val;
> > > +	ssize_t ret;
> > > +
> > > +	ret = kstrtoint_from_user(buf, count, 10, &val);
> > > +	if (ret)
> > > +		return ret;
> > > +
> > > +	mutex_lock(&debug_lock);
> > > +	idle_constraint = val;
> > > +
> > > +	if (debug_enable)
> > > +		pm_qos_update_request(&debug_qos_req, idle_constraint);
> > > +
> > > +	mutex_unlock(&debug_lock);
> > > +	return count;
> > > +}
> > > +
> > > +static ssize_t debug_idle_constraint_read(struct file *f,
> > > +		char __user *ubuf, size_t count, loff_t *ppos)
> > > +{
> > > +	char buf[32];
> > > +	int len;
> > > +
> > > +	if (*ppos)
> > > +		return 0;
> > > +
> > > +	len = sprintf(buf, "%d\n", idle_constraint);
> > > +	return simple_read_from_buffer(ubuf, count, ppos, buf, len);
> > 
> > Use the debug_lock to avoid race conditions.
> 
> Will do it.
> 
> > > +}
> > > +
> > > +static const struct file_operations debug_func_knob_fops = {
> > > +	.open	= simple_open,
> > > +	.read	= debug_func_knob_read,
> > > +	.write	= debug_func_knob_write,
> > > +};
> > > +
> > > +static const struct file_operations debug_idle_constraint_fops = {
> > > +	.open	= simple_open,
> > > +	.read	= debug_idle_constraint_read,
> > > +	.write	= debug_idle_constraint_write,
> > > +};
> > > +
> > > +static int debug_func_init(void)
> > > +{
> > > +	struct dentry *file;
> > > +	int ret;
> > > +
> > > +	/* Create debugfs node */
> > > +	debug_debugfs_dir = debugfs_create_dir("coresight_cpu_debug", NULL);
> > > +	if (!debug_debugfs_dir) {
> > > +		pr_err("%s: unable to create debugfs directory\n", __func__);
> > > +		return -ENOMEM;
> > 
> > return PTR_ERR(debug_debugfs_dir);
> 
> Here cannot use PTR_ERR(debug_debugfs_dir). If create debugfs failed
> the pointer is NULL value, so finally we will return zero value for
> PTR_ERR(debug_debugfs_dir).
> 
> [...]
> 
> > > +	}
> > > +
> > > +	file = debugfs_create_file("enable", S_IRUGO | S_IWUSR,
> > > +			debug_debugfs_dir, NULL, &debug_func_knob_fops);
> > > +	if (!file) {
> > > +		pr_err("%s: unable to create enable knob file\n", __func__);
> > > +		ret = -ENOMEM;
> > 
> > Same as above.
> > 
> > > +		goto err;
> > > +	}
> > > +
> > > +	file = debugfs_create_file("idle_constraint", S_IRUGO | S_IWUSR,
> > > +			debug_debugfs_dir, NULL, &debug_idle_constraint_fops);
> > > +	if (!file) {
> > > +		pr_err("%s: unable to create idle constraint file\n", __func__);
> > > +		ret = -ENOMEM;
> > 
> > Same as above.
> > 
> > > +		goto err;
> > > +	}
> > > +
> > > +	/* Use sysfs node to enable functionality */
> > > +	if (!debug_enable)
> > > +		return 0;
> > > +
> > > +	/* Enable debug module at boot time */
> > > +	ret = debug_enable_func();
> > > +	if (ret) {
> > > +		pr_err("%s: unable to disable debug function: %d\n",
> > > +		       __func__, ret);
> > > +		goto err;
> > > +	}
> > 
> > Use the debug_lock to avoid race conditions.
> 
> I'm struggling to understand what's race condition at here? The
> function pairs debug_func_init()/debug_func_exit() are used for
> module's probing and removing, so naturally module's probing and
> removing are sequential, right?

You are correct - void that comment.

> 
> > > +
> > > +	return 0;
> > > +
> > > +err:
> > > +	debugfs_remove_recursive(debug_debugfs_dir);
> > > +	return ret;
> > > +}
> > > +
> > > +static void debug_func_exit(void)
> > > +{
> > > +	debugfs_remove_recursive(debug_debugfs_dir);
> > > +
> > > +	/* Disable functionality if has enabled */
> > > +	if (debug_enable)
> > > +		debug_disable_func();
> > > +}
> > > +
> > > +static int debug_probe(struct amba_device *adev, const struct amba_id *id)
> > > +{
> > > +	void __iomem *base;
> > > +	struct device *dev = &adev->dev;
> > > +	struct debug_drvdata *drvdata;
> > > +	struct resource *res = &adev->res;
> > > +	struct device_node *np = adev->dev.of_node;
> > > +	int ret;
> > > +
> > > +	drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL);
> > > +	if (!drvdata)
> > > +		return -ENOMEM;
> > > +
> > > +	drvdata->cpu = np ? of_coresight_get_cpu(np) : 0;
> > > +	if (per_cpu(debug_drvdata, drvdata->cpu)) {
> > > +		dev_err(dev, "CPU's drvdata has been initialized\n");
> > 
> > Might be worth adding the CPU number in the error message.
> 
> Yeah, will add it.
> 
> [...]
> 
> > This driver doesn't call the pm_runtime_put/get() operations needed to handle the
> > debug power domain.  See the other CoreSight drivers for details. 
> 
> Sure, will do it.
> 
> > Also, from the conversation that followed the previous post we agreed that we wouldn't
> > deal with CPUidle issues in this driver.  We deal with the CPU power domain
> > using the EDPRCR register (like you did) and that's it.  System that don't honor that register
> > can use other (external) means to solve this.  As such please remove the
> > pm_qos_xyz() functions. 
> 
> From previous discussion, Mike reminds the CPU power domain design is
> quite SoC specific and usually the SoC has many different low power
> states, e.g. except CPU level and cluster level low power states, they
> also can define SoC level low power states. Any SoC with any power
> state is possible finally impact CPU power domain, so this is why I add
> this interface to let user can have the final decision based on their
> working platform.

Mike is correct but there are other ways to deal with these cases, i.e cpuidle
interface from cmd line.  

> 
> We can rely on "nohlt" and "cpuidle.off=1" in kernel command line to
> disable whole SoC low power states at boot time; or we can use sysfs
> node "echo 1 > /sys/devices/system/cpu/cpuX/cpuidle/stateX/disble" to
> disable CPU low power states at runtime. But that means we need use
> different interfaces to control CPU power domain for booting and
> runtime, it's not nice for usage.

That is a different topic altogether.

> 
> So this is why add "idle_constraint" as a central place to control
> power domain for CPU debug purpose and I also think this is more
> friendly for hardware design, e.g. some platforms can enable partial
> low power states to save power and avoid overheat after using this
> driver.
> 
> How about you think for this?

Like Sudeep pointed out we should concentrate on doing the right thing, that is
work with EDPRSR.PU, EDPRCR.COREPURQ and EDPRCR.CORENPDRQ.  Anything
outside of that becomes platform specific and can't be handled in
this driver.

> 
> Thanks,
> Leo Yan

^ permalink raw reply

* [PATCH v3 0/7] rtc: Reuse rtc-sh driver to support RZ/A1
From: Chris Brandt @ 2017-03-29 17:30 UTC (permalink / raw)
  To: Alessandro Zummo, Alexandre Belloni, Rob Herring, Mark Rutland,
	Simon Horman, Geert Uytterhoeven
  Cc: rtc-linux-/JYPxA39Uh5TLH3MbocFFw,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-renesas-soc-u79uwXL29TY76Z2rM5mHXA, Chris Brandt

Basically, the same RTC that was used in SuperH devices is now used in
RZ/A1 devices. Therefore with some slight changes, that driver can be
reused.

Additionally, since ARM architectures require Device Tree configurations,
device tree support has been added.

This was tested on RZ/A1H RSK board. Once the correct time was programmed
into the RTC running off a 32.768kHz crystal, you can retain an accurate
system clock between reboots (hard and soft) until power is finally removed
from the board (there is no batter backup on the RSK board).

v3:
* remove 'power-domains' from example in documentation
* update commit message subject and body of dt-bindings

v2:
* removed HAVE_CLK from Kconfig
* when using DT, look for "fck" instead of "rtc0"
* changed (res == NULL) to (!res)
* added Reviewed-by for rtc-sh.c
* added interrupt-names and clock-names
* clocks now include counting sources
* added clock-names "fck", "rtc_x1", "rtc_x3", "extal"
* changed 'is a RTC' to 'is an RTC' in commit message
* changed "rtc0" to "rtc"
* Added clock nodes for RTC_X1 and RTC_X3 pin inputs
* each commit now has a log message


Chris Brandt (7):
  rtc: rtc-sh: add support for rza series
  dt-bindings: rtc: document the rtc-sh bindings
  ARM: dts: r7s72100: add rtc clock to device tree
  ARM: dts: r7s72100: add RTC_X clock inputs to device tree
  ARM: dts: r7s72100: add rtc to device tree
  ARM: dts: rskrza1: set rtc_x1 clock value
  ARM: dts: rskrza1: add rtc DT support

 Documentation/devicetree/bindings/rtc/rtc-sh.txt | 28 ++++++++++++++++++
 arch/arm/boot/dts/r7s72100-rskrza1.dts           |  8 +++++
 arch/arm/boot/dts/r7s72100.dtsi                  | 37 ++++++++++++++++++++++++
 drivers/rtc/Kconfig                              |  4 +--
 drivers/rtc/rtc-sh.c                             | 33 +++++++++++++++++----
 include/dt-bindings/clock/r7s72100-clock.h       |  3 ++
 6 files changed, 106 insertions(+), 7 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/rtc/rtc-sh.txt

-- 
2.11.0


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* [PATCH v3 1/7] rtc: rtc-sh: add support for rza series
From: Chris Brandt @ 2017-03-29 17:30 UTC (permalink / raw)
  To: Alessandro Zummo, Alexandre Belloni, Rob Herring, Mark Rutland,
	Simon Horman, Geert Uytterhoeven
  Cc: rtc-linux-/JYPxA39Uh5TLH3MbocFFw,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-renesas-soc-u79uwXL29TY76Z2rM5mHXA, Chris Brandt
In-Reply-To: <20170329173035.67477-1-chris.brandt-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>

This same RTC is used in RZ/A series MPUs, therefore with some slight
changes, this driver can be reused. Additionally, since ARM architectures
require Device Tree configurations, device tree support has been added.

Signed-off-by: Chris Brandt <chris.brandt-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
Reviewed-by: Geert Uytterhoeven <geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ@public.gmane.org>
---
v2:
* removed HAVE_CLK from Kconfig
* when using DT, look for "fck" instead of "rtc0"
* changed (res == NULL) to (!res)
* added Reviewed-by
---
 drivers/rtc/Kconfig  |  4 ++--
 drivers/rtc/rtc-sh.c | 33 ++++++++++++++++++++++++++++-----
 2 files changed, 30 insertions(+), 7 deletions(-)

diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index ee1b0e9dde79..c2d1f0ed47a1 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -1303,10 +1303,10 @@ config RTC_DRV_SA1100
 
 config RTC_DRV_SH
 	tristate "SuperH On-Chip RTC"
-	depends on SUPERH && HAVE_CLK
+	depends on SUPERH || ARCH_RENESAS
 	help
 	  Say Y here to enable support for the on-chip RTC found in
-	  most SuperH processors.
+	  most SuperH processors. This RTC is also found in RZ/A SoCs.
 
 	  To compile this driver as a module, choose M here: the
 	  module will be called rtc-sh.
diff --git a/drivers/rtc/rtc-sh.c b/drivers/rtc/rtc-sh.c
index c626e43a9cbb..00b396e96cbe 100644
--- a/drivers/rtc/rtc-sh.c
+++ b/drivers/rtc/rtc-sh.c
@@ -27,7 +27,15 @@
 #include <linux/log2.h>
 #include <linux/clk.h>
 #include <linux/slab.h>
+#ifdef CONFIG_SUPERH
 #include <asm/rtc.h>
+#else
+/* Default values for RZ/A RTC */
+#define rtc_reg_size		sizeof(u16)
+#define RTC_BIT_INVERTED        0	/* no chip bugs */
+#define RTC_CAP_4_DIGIT_YEAR    (1 << 0)
+#define RTC_DEF_CAPABILITIES    RTC_CAP_4_DIGIT_YEAR
+#endif
 
 #define DRV_NAME	"sh-rtc"
 
@@ -570,6 +578,8 @@ static int __init sh_rtc_probe(struct platform_device *pdev)
 	rtc->alarm_irq = platform_get_irq(pdev, 2);
 
 	res = platform_get_resource(pdev, IORESOURCE_IO, 0);
+	if (!res)
+		res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (unlikely(res == NULL)) {
 		dev_err(&pdev->dev, "No IO resource\n");
 		return -ENOENT;
@@ -587,12 +597,15 @@ static int __init sh_rtc_probe(struct platform_device *pdev)
 	if (unlikely(!rtc->regbase))
 		return -EINVAL;
 
-	clk_id = pdev->id;
-	/* With a single device, the clock id is still "rtc0" */
-	if (clk_id < 0)
-		clk_id = 0;
+	if (!pdev->dev.of_node) {
+		clk_id = pdev->id;
+		/* With a single device, the clock id is still "rtc0" */
+		if (clk_id < 0)
+			clk_id = 0;
 
-	snprintf(clk_name, sizeof(clk_name), "rtc%d", clk_id);
+		snprintf(clk_name, sizeof(clk_name), "rtc%d", clk_id);
+	} else
+		snprintf(clk_name, sizeof(clk_name), "fck");
 
 	rtc->clk = devm_clk_get(&pdev->dev, clk_name);
 	if (IS_ERR(rtc->clk)) {
@@ -608,6 +621,8 @@ static int __init sh_rtc_probe(struct platform_device *pdev)
 	clk_enable(rtc->clk);
 
 	rtc->capabilities = RTC_DEF_CAPABILITIES;
+
+#ifdef CONFIG_SUPERH
 	if (dev_get_platdata(&pdev->dev)) {
 		struct sh_rtc_platform_info *pinfo =
 			dev_get_platdata(&pdev->dev);
@@ -618,6 +633,7 @@ static int __init sh_rtc_probe(struct platform_device *pdev)
 		 */
 		rtc->capabilities |= pinfo->capabilities;
 	}
+#endif
 
 	if (rtc->carry_irq <= 0) {
 		/* register shared periodic/carry/alarm irq */
@@ -738,10 +754,17 @@ static int sh_rtc_resume(struct device *dev)
 
 static SIMPLE_DEV_PM_OPS(sh_rtc_pm_ops, sh_rtc_suspend, sh_rtc_resume);
 
+static const struct of_device_id sh_rtc_of_match[] = {
+	{ .compatible = "renesas,sh-rtc", },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, sh_rtc_of_match);
+
 static struct platform_driver sh_rtc_platform_driver = {
 	.driver		= {
 		.name	= DRV_NAME,
 		.pm	= &sh_rtc_pm_ops,
+		.of_match_table = sh_rtc_of_match,
 	},
 	.remove		= __exit_p(sh_rtc_remove),
 };
-- 
2.11.0


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* [PATCH v3 2/7] dt-bindings: rtc: document the rtc-sh bindings
From: Chris Brandt @ 2017-03-29 17:30 UTC (permalink / raw)
  To: Alessandro Zummo, Alexandre Belloni, Rob Herring, Mark Rutland,
	Simon Horman, Geert Uytterhoeven
  Cc: rtc-linux-/JYPxA39Uh5TLH3MbocFFw,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-renesas-soc-u79uwXL29TY76Z2rM5mHXA, Chris Brandt
In-Reply-To: <20170329173035.67477-1-chris.brandt-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>

Add the binding documentation for rtc-sh which is an RTC for
SuperH and RZ/A SoCs.

Signed-off-by: Chris Brandt <chris.brandt-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
---
v3:
* remove 'power-domains' from example
* update commit message subject and body
v2:
* added interrupt-names and clock-names
* clocks now include counting sources
* changed 'is a RTC' to 'is an RTC' in commit message
---
 Documentation/devicetree/bindings/rtc/rtc-sh.txt | 28 ++++++++++++++++++++++++
 1 file changed, 28 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/rtc/rtc-sh.txt

diff --git a/Documentation/devicetree/bindings/rtc/rtc-sh.txt b/Documentation/devicetree/bindings/rtc/rtc-sh.txt
new file mode 100644
index 000000000000..7676c7d28874
--- /dev/null
+++ b/Documentation/devicetree/bindings/rtc/rtc-sh.txt
@@ -0,0 +1,28 @@
+* Real Time Clock for Renesas SH and ARM SoCs
+
+Required properties:
+- compatible: Should be "renesas,r7s72100-rtc" and "renesas,sh-rtc" as a
+  fallback.
+- reg: physical base address and length of memory mapped region.
+- interrupts: 3 interrupts for alarm, period, and carry.
+- interrupt-names: The interrupts should be labeled as "alarm", "period", and
+  "carry".
+- clocks: The functional clock source for the RTC controller must be listed
+  first (if exists). Additionally, potential clock counting sources are to be
+  listed.
+- clock-names: The functional clock must be labeled as "fck". Other clocks
+  may be named in accordance to the SoC hardware manuals.
+
+
+Example:
+rtc: rtc@fcff1000 {
+	compatible = "renesas,r7s72100-rtc", "renesas,sh-rtc";
+	reg = <0xfcff1000 0x2e>;
+	interrupts = <GIC_SPI 276 IRQ_TYPE_EDGE_RISING
+		      GIC_SPI 277 IRQ_TYPE_EDGE_RISING
+		      GIC_SPI 278 IRQ_TYPE_EDGE_RISING>;
+	interrupt-names = "alarm", "period", "carry";
+	clocks = <&mstp6_clks R7S72100_CLK_RTC>, <&rtc_x1_clk>,
+		 <&rtc_x3_clk>, <&extal_clk>;
+	clock-names = "fck", "rtc_x1", "rtc_x3", "extal";
+};
-- 
2.11.0


-- 
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
--- 
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply related

* [PATCH v3 3/7] ARM: dts: r7s72100: add rtc clock to device tree
From: Chris Brandt @ 2017-03-29 17:30 UTC (permalink / raw)
  To: Alessandro Zummo, Alexandre Belloni, Rob Herring, Mark Rutland,
	Simon Horman, Geert Uytterhoeven
  Cc: rtc-linux-/JYPxA39Uh5TLH3MbocFFw,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-renesas-soc-u79uwXL29TY76Z2rM5mHXA, Chris Brandt
In-Reply-To: <20170329173035.67477-1-chris.brandt-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>

Add the realtime clock functional clock source.

Signed-off-by: Chris Brandt <chris.brandt-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
Reviewed-by: Geert Uytterhoeven <geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ@public.gmane.org>
---
v2:
* changed "rtc0" to "rtc"
* added Reviewed-by
---
 arch/arm/boot/dts/r7s72100.dtsi            | 9 +++++++++
 include/dt-bindings/clock/r7s72100-clock.h | 3 +++
 2 files changed, 12 insertions(+)

diff --git a/arch/arm/boot/dts/r7s72100.dtsi b/arch/arm/boot/dts/r7s72100.dtsi
index 34994afa9d15..6a1448be4e10 100644
--- a/arch/arm/boot/dts/r7s72100.dtsi
+++ b/arch/arm/boot/dts/r7s72100.dtsi
@@ -117,6 +117,15 @@
 			clock-output-names = "ostm0", "ostm1";
 		};
 
+		mstp6_clks: mstp6_clks@fcfe042c {
+			#clock-cells = <1>;
+			compatible = "renesas,r7s72100-mstp-clocks", "renesas,cpg-mstp-clocks";
+			reg = <0xfcfe042c 4>;
+			clocks = <&p0_clk>;
+			clock-indices = <R7S72100_CLK_RTC>;
+			clock-output-names = "rtc";
+		};
+
 		mstp7_clks: mstp7_clks@fcfe0430 {
 			#clock-cells = <1>;
 			compatible = "renesas,r7s72100-mstp-clocks", "renesas,cpg-mstp-clocks";
diff --git a/include/dt-bindings/clock/r7s72100-clock.h b/include/dt-bindings/clock/r7s72100-clock.h
index cd2ed5194255..bc256d31099a 100644
--- a/include/dt-bindings/clock/r7s72100-clock.h
+++ b/include/dt-bindings/clock/r7s72100-clock.h
@@ -29,6 +29,9 @@
 #define R7S72100_CLK_OSTM0	1
 #define R7S72100_CLK_OSTM1	0
 
+/* MSTP6 */
+#define R7S72100_CLK_RTC	0
+
 /* MSTP7 */
 #define R7S72100_CLK_ETHER	4
 
-- 
2.11.0


-- 
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
--- 
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply related

* [PATCH v3 4/7] ARM: dts: r7s72100: add RTC_X clock inputs to device tree
From: Chris Brandt @ 2017-03-29 17:30 UTC (permalink / raw)
  To: Alessandro Zummo, Alexandre Belloni, Rob Herring, Mark Rutland,
	Simon Horman, Geert Uytterhoeven
  Cc: rtc-linux-/JYPxA39Uh5TLH3MbocFFw,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-renesas-soc-u79uwXL29TY76Z2rM5mHXA, Chris Brandt
In-Reply-To: <20170329173035.67477-1-chris.brandt-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>

Add the RTC clocks to device tree. The frequencies must be fixed values
according to the hardware manual.

Signed-off-by: Chris Brandt <chris.brandt-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
---
 arch/arm/boot/dts/r7s72100.dtsi | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/arch/arm/boot/dts/r7s72100.dtsi b/arch/arm/boot/dts/r7s72100.dtsi
index 6a1448be4e10..632a6a481be8 100644
--- a/arch/arm/boot/dts/r7s72100.dtsi
+++ b/arch/arm/boot/dts/r7s72100.dtsi
@@ -51,6 +51,20 @@
 			clock-frequency = <0>;
 		};
 
+		rtc_x1_clk: rtc_x1 {
+			#clock-cells = <0>;
+			compatible = "fixed-clock";
+			/* If clk present, value must be set by board to 32678 */
+			clock-frequency = <0>;
+		};
+
+		rtc_x3_clk: rtc_x3 {
+			#clock-cells = <0>;
+			compatible = "fixed-clock";
+			/* If clk present, value must be set by board to 4000000 */
+			clock-frequency = <0>;
+		};
+
 		/* Fixed factor clocks */
 		b_clk: b {
 			#clock-cells = <0>;
-- 
2.11.0


-- 
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
--- 
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply related

* [PATCH v3 5/7] ARM: dts: r7s72100: add rtc to device tree
From: Chris Brandt @ 2017-03-29 17:30 UTC (permalink / raw)
  To: Alessandro Zummo, Alexandre Belloni, Rob Herring, Mark Rutland,
	Simon Horman, Geert Uytterhoeven
  Cc: rtc-linux, devicetree, linux-renesas-soc, Chris Brandt
In-Reply-To: <20170329173035.67477-1-chris.brandt@renesas.com>

Add the realtime clock device node.

Signed-off-by: Chris Brandt <chris.brandt@renesas.com>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
v2:
* added interrupt-names
* added clock counting sources
* added clock-names "fck", "rtc_x1", "rtc_x3", "extal"
---
 arch/arm/boot/dts/r7s72100.dtsi | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/arch/arm/boot/dts/r7s72100.dtsi b/arch/arm/boot/dts/r7s72100.dtsi
index 632a6a481be8..f74a8ec385b6 100644
--- a/arch/arm/boot/dts/r7s72100.dtsi
+++ b/arch/arm/boot/dts/r7s72100.dtsi
@@ -652,4 +652,18 @@
 		power-domains = <&cpg_clocks>;
 		status = "disabled";
 	};
+
+	rtc: rtc@fcff1000 {
+		compatible = "renesas,r7s72100-rtc", "renesas,sh-rtc";
+		reg = <0xfcff1000 0x2e>;
+		interrupts = <GIC_SPI 276 IRQ_TYPE_EDGE_RISING
+			      GIC_SPI 277 IRQ_TYPE_EDGE_RISING
+			      GIC_SPI 278 IRQ_TYPE_EDGE_RISING>;
+		interrupt-names = "alarm", "period", "carry";
+		clocks = <&mstp6_clks R7S72100_CLK_RTC>, <&rtc_x1_clk>,
+			 <&rtc_x3_clk>, <&extal_clk>;
+		clock-names = "fck", "rtc_x1", "rtc_x3", "extal";
+		power-domains = <&cpg_clocks>;
+		status = "disabled";
+	};
 };
-- 
2.11.0

^ permalink raw reply related

* [PATCH v3 6/7] ARM: dts: rskrza1: set rtc_x1 clock value
From: Chris Brandt @ 2017-03-29 17:30 UTC (permalink / raw)
  To: Alessandro Zummo, Alexandre Belloni, Rob Herring, Mark Rutland,
	Simon Horman, Geert Uytterhoeven
  Cc: rtc-linux-/JYPxA39Uh5TLH3MbocFFw,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-renesas-soc-u79uwXL29TY76Z2rM5mHXA, Chris Brandt
In-Reply-To: <20170329173035.67477-1-chris.brandt-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>

Enable the 32.768 kHz RTC_X1 clock by setting the frequency value to
non-zero.

Signed-off-by: Chris Brandt <chris.brandt-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
---
 arch/arm/boot/dts/r7s72100-rskrza1.dts | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/boot/dts/r7s72100-rskrza1.dts b/arch/arm/boot/dts/r7s72100-rskrza1.dts
index 02b59c5b3c53..cab5857bfb41 100644
--- a/arch/arm/boot/dts/r7s72100-rskrza1.dts
+++ b/arch/arm/boot/dts/r7s72100-rskrza1.dts
@@ -43,6 +43,10 @@
 	clock-frequency = <48000000>;
 };
 
+&rtc_x1_clk {
+	clock-frequency = <32768>;
+};
+
 &mtu2 {
 	status = "okay";
 };
-- 
2.11.0


-- 
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
--- 
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply related

* [PATCH v3 7/7] ARM: dts: rskrza1: add rtc DT support
From: Chris Brandt @ 2017-03-29 17:30 UTC (permalink / raw)
  To: Alessandro Zummo, Alexandre Belloni, Rob Herring, Mark Rutland,
	Simon Horman, Geert Uytterhoeven
  Cc: rtc-linux, devicetree, linux-renesas-soc, Chris Brandt
In-Reply-To: <20170329173035.67477-1-chris.brandt@renesas.com>

Enable the realtime clock.

Signed-off-by: Chris Brandt <chris.brandt@renesas.com>
---
 arch/arm/boot/dts/r7s72100-rskrza1.dts | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/boot/dts/r7s72100-rskrza1.dts b/arch/arm/boot/dts/r7s72100-rskrza1.dts
index cab5857bfb41..72df20a04320 100644
--- a/arch/arm/boot/dts/r7s72100-rskrza1.dts
+++ b/arch/arm/boot/dts/r7s72100-rskrza1.dts
@@ -73,6 +73,10 @@
 	status = "okay";
 };
 
+&rtc {
+	status = "okay";
+};
+
 &scif2 {
 	status = "okay";
 };
-- 
2.11.0

^ permalink raw reply related

* Re: [PATCH v2 7/8] bus: brcmstb_gisb: add notifier handling
From: Doug Berger @ 2017-03-29 17:39 UTC (permalink / raw)
  To: Mark Rutland
  Cc: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, catalin.marinas-5wv7dgnIgG8,
	will.deacon-5wv7dgnIgG8, computersforpeace-Re5JQEeQqe8AvxtiuMwx3w,
	gregory.0xf0-Re5JQEeQqe8AvxtiuMwx3w,
	f.fainelli-Re5JQEeQqe8AvxtiuMwx3w,
	bcm-kernel-feedback-list-dY08KVG/lbpWk0Htik3J/w,
	wangkefeng.wang-hv44wF8Li93QT0dZR+AlfA, james.morse-5wv7dgnIgG8,
	mingo-DgEjT+Ai2ygdnm+yROfE0A,
	sandeepa.s.prabhu-Re5JQEeQqe8AvxtiuMwx3w,
	shijie.huang-5wv7dgnIgG8, linus.walleij-QSEj5FYQhm4dnm+yROfE0A,
	treding-DDmLM1+adcrQT0dZR+AlfA, jonathanh-DDmLM1+adcrQT0dZR+AlfA,
	olof-nZhT3qVonbNeoWH0uzbU5w, mirza.krak-Re5JQEeQqe8AvxtiuMwx3w,
	suzuki.poulose-5wv7dgnIgG8, bgolaszewski-rdvid1DuHRBWk0Htik3J/w,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <20170329101357.GB23442@leverpostej>

On 03/29/2017 03:13 AM, Mark Rutland wrote:

snip

>> +/*
>> + * Dump out gisb errors on die or panic.
>> + */
>> +static int dump_gisb_error(struct notifier_block *self, unsigned long v,
>> +			   void *p)
>> +{
>> +	struct brcmstb_gisb_arb_device *gdev;
>> +
>> +	/* iterate over each GISB arb registered handlers */
>> +	list_for_each_entry(gdev, &brcmstb_gisb_arb_device_list, next)
>> +		brcmstb_gisb_arb_decode_addr(gdev, "async abort");
>> +
>> +	return 0;
> 
> I think this should be NOTIFY_OK.
> 
I used dump_mem_limit() as a template and didn't catch this (work to
do...).  Upon review I think I would prefer NOTIFY_DONE since this call
is opportunistic (i.e. it is taking the opportunity to check whether
additional diagnostic data is available to display) and has no interest
in affecting the overall handling of the event.

>> +}
>> +
>> +static struct notifier_block gisb_error_notifier = {
>> +	.notifier_call = dump_gisb_error,
>> +};
>> +
>>  static DEVICE_ATTR(gisb_arb_timeout, S_IWUSR | S_IRUGO,
>>  		gisb_arb_get_timeout, gisb_arb_set_timeout);
>>  
>> @@ -408,6 +429,12 @@ static int __init brcmstb_gisb_arb_probe(struct platform_device *pdev)
>>  	board_be_handler = brcmstb_bus_error_handler;
>>  #endif
>>  
>> +	if (list_is_singular(&brcmstb_gisb_arb_device_list)) {
>> +		register_die_notifier(&gisb_error_notifier);
>> +		atomic_notifier_chain_register(&panic_notifier_list,
>> +					       &gisb_error_notifier);
> 
> I don't think this is quite right. A notifier_block can only be
> registered to one notifier chain at a time, and this has the potential
> to corrupt both chains.
> 
A VERY good point thanks for pointing this out.

> I also think you only need to register the panic notifier. An SError
> should always result in a panic.
> 
That was my initial thought as well.  However, testing revealed that the
bad mode Oops actually exits the user space process and doesn't reach
the panic so there was no helpful diagnostic message.  This may be in
line with your comments about insufficient fatality of failures in PATCH
v2 6/8, but it actually is more in line with our desired behavior for
the aborted write.  Setting the notify on die gave us the result we are
looking for, but as noted above I should have created a separate notifier.

I had hoped that the same approach (i.e. die notifier) would remove the
need for PATCH v2 6/8 as well, but I found that the Unhandled fault
error didn't actually die from user mode.

> Thanks,
> Mark.
> 
Thank you for your help with this,
    Doug
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [RFC PATCH] of: Fix DMA configuration for non-DT masters
From: Robin Murphy @ 2017-03-29 17:42 UTC (permalink / raw)
  To: Oza Oza
  Cc: Linux IOMMU, devicetree-u79uwXL29TY76Z2rM5mHXA,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <CAMSpPPcSWxua_6ECZd4iEbTiAYk-KtapBDcOUxNdGaQkFF-LOQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On 29/03/17 06:46, Oza Oza wrote:
> On Wed, Mar 29, 2017 at 10:23 AM, Oza Oza <oza.oza-dY08KVG/lbpWk0Htik3J/w@public.gmane.org> wrote:
>> On Wed, Mar 29, 2017 at 12:27 AM, Robin Murphy <robin.murphy-5wv7dgnIgG8@public.gmane.org> wrote:
>>> For PCI masters not represented in DT, we pass the OF node of their
>>> associated host bridge to of_dma_configure(), such that they can inherit
>>> the appropriate DMA configuration from whatever is described there.
>>> Unfortunately, whilst this has worked for the "dma-coherent" property,
>>> it turns out to miss the case where the host bridge node has a non-empty
>>> "dma-ranges", since nobody is expecting the 'device' to be a bus itself.
>>>
>>> It transpires, though, that the de-facto interface since the prototype
>>> change in 1f5c69aa51f9 ("of: Move of_dma_configure() to device.c to help
>>> re-use") is very clear-cut: either the master_np argument is redundant
>>> with dev->of_node, or dev->of_node is NULL and master_np is the relevant
>>> parent bus. Let's ratify that behaviour, then teach the whole
>>> of_dma_configure() pipeline to cope with both cases properly.
>>>
>>> Signed-off-by: Robin Murphy <robin.murphy-5wv7dgnIgG8@public.gmane.org>

[...]

>>
>> pci and memory mapped device world is different.

???

No they aren't. There is nothing magic about PCI. PCI *is* a
memory-mapped bus.

The only PCI-specific aspect here is the Linux code path which passes a
host controller node into of_dma_configure() where the latter expects a
node for the actual endpoint device. It managed to work for
"dma-coherent", because that may appear either directly on a device node
or on any of its parent buses, but "dma-ranges" is *only* valid for DT
nodes representing buses, thus reveals that using a parent bus to stand
in for a device isn't actually correct. I only posted that horrible hack
patch to prove the point that having a child node to represent the
actual device is indeed sufficient to make of_dma_configure() work
correctly for PCI masters as-is (modulo the other issues).

> of course if you talk
>> from iommu perspective, all the master are same for IOMMU

I don't understand what you mean by that. There's nothing about IOMMUs
here, it's purely about parsing DT properties correctly.

>> but the original intention of the patch is to try to solve 2 problems.
>> please refer to https://lkml.org/lkml/2017/3/29/10

One patch should not solve two separate problems anyway. Taking a step
back, there are at least 3 things that this discussion has brought up:

1) The way in which we call of_dma_configure() for PCI devices causes
the "dma-ranges" property on PCI host controllers to be ignored.

2) of_dma_get_range() only considers the first entry in any "dma-ranges"
property.

3) When of_dma_configure() does set a DMA mask, there is nothing on
arm64 and other architectures to prevent drivers overriding that with a
wider mask later.

Those are 3 separate problems, to solve with 3 or more separate patches,
and I have deliberately put the second and third to one side for the
moment. This patch fixes problem 1.

>> 1) expose generic API for pci world clients to configure their
>> dma-ranges. right now there is none.
>> 2) same API can be used by IOMMU to derive dma_mask.
>>
>> while the current patch posted to handle dma-ranges for both memory
>> mapped and pci devices, which I think is overdoing.

No, of_dma_configure() handles the "dma-ranges" property as it is
defined in the DT spec to describe the mapping between a child bus
address space and a parent bus address space. Whether those
memory-mapped parent and child buses are PCI, ISA, AMBA, HyperTransport,
or anything else is irrelevant other than for the encoding of the
address specifiers. All this patch does is sort out the cases where we
have a real device node to start at, from the cases where we don't and
so start directly at the device's parent bus node instead.

>> besides we have different configuration of dma-ranges based on iommu
>> is enabled or not enabled.

That doesn't sound right, unless you mean the firmware actually programs
the host controller's AXI bridge differently for system configurations
where the IOMMU is expected to be used or not? (and even then, I don't
really see why it would be necessary to do that...)

>>  #define PCIE_DMA_RANGES dma-ranges = <0x43000000 0x00 0x80000000 0x00
>> 0x80000000 0x00 0x80000000 \
>>                                       0x43000000 0x08 0x00000000 0x08
>> 0x00000000 0x08 0x00000000 \
>>                                       0x43000000 0x80 0x00000000 0x80
>> 0x00000000 0x40 0x00000000>;
>> Not sure if this patch will consider above dma-ranges.
>>
>> my suggestion is to leave existing of_dma_get_range as is, and have
>> new function for pci world as discussed in
>> please refer to https://lkml.org/lkml/2017/3/29/10

And then we keep adding new functions to do the exact same thing for
every other discoverable bus type whose bridge is be described in DT? I
fail to see how that is in any way better than simply fixing the
existing code to work as it was intended to.

of_dma_get_ranges() uses of_translate_dma_address() just the same way as
existing PowerPC PCI code does, which further disproves your assertion
that parsing PCI addresses is somehow special - it's really only a
matter of getting the right number of address cells in order to to read
a child address to give to of_translate_dma_address() in the first
place. Incidentally, I now notice that the proposed
of_pci_get_dma_ranges() is incomplete as it doesn't use
of_translate_dma_address() or otherwise traverse upwards through the
dma-ranges of any further parent buses.

>>
>> Regards,
>> Oza.
> 
> also I see that, in case of multiple ranges of_dma_get_range doesnt handle that.
> and also it is not meant to handle.

Yes, the existing code doesn't handle multiple dma-ranges entries,
because nobody's had the need to implement that so far, and this patch
does not change that because it's fixing a separate problem.

Now, of course of_dma_get_range() *should* be capable of handling
multiple entries regardless of this patch, and I'm going to write *that*
patch right now (it's going to be a case of adding a handful of lines
which probably won't even conflict with this one at all). If we had a
bunch of different range parsing functions, we'd then have to duplicate
the equivalent logic across all of them, which is clearly undesirable
when it can easily be avoided altogether.

Robin.

> so with this patch will return wrong size and hence wrong dma_mask.
> having said, I think it is better to separate pci world dma-ranges
> function on of_pci.c
> 
> for which my discussion with Rob already, (same thread)
> https://lkml.org/lkml/2017/3/29/10
> Waiting for Rob's viewpoint on this.
> 
> 
> Regards,
> Oza.
> 

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* [PATCH v4 00/23] FSI device driver implementation
From: Christopher Bostic @ 2017-03-29 17:43 UTC (permalink / raw)
  To: robh+dt, mark.rutland, linux, rostedt, mingo, gregkh, devicetree,
	linux-arm-kernel
  Cc: Christopher Bostic, joel, linux-kernel, andrew, alistair, benh

Implementation of the IBM 'Flexible Support Interface' (FSI) bus device
driver. FSI is a high fan out serial bus consisting of a clock and a serial
data line capable of running at speeds up to 166 MHz.

This set provides the basic framework to add FSI extensions to the
Linux bus and device models. Master specific implementations are
defined to utilize the core FSI function.

In Linux, we have a core FSI "bus type", along with drivers for FSI
masters and engines.

The FSI master drivers expose a read/write interface to the bus address
space. The master drivers are under drivers/fsi/fsi-master-*.c.

The core handles probing and discovery of slaves and slave
engines, using those read/write interfaces. It is responsible for
creating the endpoint Linux devices corresponding to the discovered
engines on each slave.

Slave engines are identified by an 'engine' type, and an optional
version. Engine, a.k.a. client, drivers are matched and bound to these
engines during discovery.

This patch set does not include extended FSI function such as:
    *  Cascaded master support
    *  Application layer hot plug notification
    *  Application layer FSI bus status interface

Common FSI terminology:

* Master
    Controller of the FSI bus.  Only the master is allowed to control the
    clock line and is the initiator of all transactions on a bus.

* Slave
    The receiver or target of a master initiated transaction.  The slave
    cannot initiate communications on a bus and must respond to any
    master requests for data.

* CFAM
    Stands for Common Field replaceable unit Access Macro.  A CFAM is an
    ASIC residing in any device requiring FSI communications. CFAMs
    consist of an array of hardware 'engines' used for various purposes.
    I2C masters, UARTs, General Purpose IO hardware are common types of
    these engines.

* Configuration Space / Table
    A table contained at the beginning of each CFAM address space.
    This table lists information such as the CFAM's ID, which engine types
    and versions it has available, as well as its addressing range.

* FSI Engine driver
    A device driver that registers with the FSI core so that it can access
    devices it owns on an FSI bus.

* Hub
    An FSI master that connects to an upstream 'primary' master allowing
    high fanout of target devices.

----
Changes in v4:
   - endianness: the _read() and _write() APIs are now all *bus endian*,
     so will be the same on all platforms (the previous fsi patches
     exposed as (BMC/FSP) CPU endian, which is variable).
   - device tree: Remove the "ibm," prefix for the fsi core and GPIO
     master compatibility strings, as they're not describing
     IBM-specific
   - device model: Create separate struct devices for each FSI
     master, which fits better with the Linux device model, and allows
     addition of sysfs attributes that are implemented by the fsi core
   - sysfs: there are now sysfs facilities for break and term. Raw
     file supports reads and writes of arbitrary sizes.
   - GPIO master: split the xfer() logic out a little, so that the
     response handling & DPOLL retry mechanism is more obvious
   - GPIO master: simplifications for message construction
   - GPIO master: fixes for some CRC calculations
   - GPIO master: issue TERM in response to DPOLL busy-loops
   - Error handling: rather than handle errors on (potentially) an entire
     cascaded read or write, the error handling is now down on a per-slave
     basis, where we try to reestablish communication in a more "gradual"
     manner, rather than sending a break immediately. May need to add a
     hook to percolate error recovery up to a slave's master but no need
     seen for that at present.
   - Hub master: this is now implemented as a fsi engine driver, as the
     fsi_slave_{read,write}() functions are exported (and the port count
     is available in the hMFSI configuration register)
     This means we need fewer special-cases in the fsi core.
   - Tracepoints: Add tracepoints for FSI core read & write, and another
     set for low-level GPIO in/out operations.

Changes in v3:
    - Patch set contained an invalid 18/18 test patch not
      meant for community review, corrected.

Changes in v2:
    - Change from atomic global for master number to ida simple
      interface.
    - Add valid pointer checks on register and unregister utils.
    - Move CRC calculation utilities out of driver to lib path.
    - Clean up white space issues.
    - Remove added list management of master devices and use
      instead the device_for_each_child method available in the
      bus.
    - Add new patch to document FSI bus functionality.
    - Add new patch documenting FSI gpio master.
    - Rearrage patch set to have documentation earlier than code
      implementing it.
    - Document all compatible strings used in device tree bindings.
    - Elaborate documentation definition of FSI GPIO master.
    - Describe in more detail what each GPIO FSI master pin is for.
    - Re-order compatible strings in example binding so that most
      specific device comes first.
    - Indicate proper activation order of all FSI GPIO master pins.
    - Fix an unmatched '>' bracket in the example for binding.
    - Bracket each element of the example bindings individually.
    - Add new patch documenting sysfs-bus-fsi attributes.
    - Merge FSI GPIO master init into probe function.
    - Set pin initial values at time of pin request.
    - Assign value of master->master.dev at probe time.
    - Use get_optional interface for all optional GPIO pins.

Chris Bostic (9):
  drivers/fsi: Set up links for slave communication
  drivers/fsi: Set slave SMODE to init communication
  drivers/fsi: Add master unscan
  drivers/fsi: Add documentation for GPIO bindings
  drivers/fsi: Add client driver register utilities
  drivers/fsi: Document FSI master sysfs files in ABI
  drivers/fsi: Add GPIO based FSI master
  drivers/fsi: Add SCOM FSI client device driver
  drivers/fsi: Add hub master support

Jeremy Kerr (14):
  drivers/fsi: Add fsi master definition
  drivers/fsi: Add slave definition
  drivers/fsi: Add empty master scan
  drivers/fsi: Add crc4 helpers
  drivers/fsi: Add slave & master read/write APIs
  drivers/fsi: Implement slave initialisation
  drivers/fsi: scan slaves & register devices
  drivers/fsi: Add device read/write/peek API
  drivers/fsi: Add sysfs files for FSI master & slave accesses
  drivers/fsi: expose direct-access slave API
  drivers/fsi: Add tracepoints for low-level operations
  drivers/fsi: Add error handling for slave communication errors
  drivers/fsi/gpio: Add tracepoints for GPIO master
  drivers/fsi: Use asynchronous slave mode

 Documentation/ABI/testing/sysfs-bus-fsi            |   6 +
 .../devicetree/bindings/fsi/fsi-master-gpio.txt    |  24 +
 arch/arm/boot/dts/aspeed-bmc-opp-palmetto.dts      |  10 +
 arch/arm/boot/dts/aspeed-bmc-opp-romulus.dts       |  12 +
 drivers/fsi/Kconfig                                |  26 +
 drivers/fsi/Makefile                               |   3 +
 drivers/fsi/fsi-core.c                             | 835 +++++++++++++++++++++
 drivers/fsi/fsi-master-gpio.c                      | 624 +++++++++++++++
 drivers/fsi/fsi-master-hub.c                       | 327 ++++++++
 drivers/fsi/fsi-master.h                           |  64 ++
 drivers/fsi/fsi-scom.c                             | 263 +++++++
 include/linux/fsi.h                                |  35 +-
 include/trace/events/fsi.h                         | 127 ++++
 include/trace/events/fsi_master_gpio.h             |  68 ++
 14 files changed, 2423 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-fsi
 create mode 100644 Documentation/devicetree/bindings/fsi/fsi-master-gpio.txt
 create mode 100644 drivers/fsi/fsi-master-gpio.c
 create mode 100644 drivers/fsi/fsi-master-hub.c
 create mode 100644 drivers/fsi/fsi-master.h
 create mode 100644 drivers/fsi/fsi-scom.c
 create mode 100644 include/trace/events/fsi.h
 create mode 100644 include/trace/events/fsi_master_gpio.h

-- 
1.8.2.2

^ permalink raw reply

* [PATCH v4 01/23] drivers/fsi: Add fsi master definition
From: Christopher Bostic @ 2017-03-29 17:43 UTC (permalink / raw)
  To: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	linux-I+IVW8TIWO2tmTQ+vhA3Yw, rostedt-nx8X9YLhiw1AfugRpC6u6w,
	mingo-H+wXaHxf7aLQT0dZR+AlfA,
	gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
  Cc: Jeremy Kerr, joel-U3u1mxZcP9KHXe+LvDLADg,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, andrew-zrmu5oMJ5Fs,
	alistair-Y4h6yKqj69EXC2x5gXVKYQ,
	benh-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r, Chris Bostic
In-Reply-To: <20170329174340.89109-1-cbostic-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>

From: Jeremy Kerr <jk-mnsaURCQ41sdnm+yROfE0A@public.gmane.org>

Add a `struct fsi_master` to represent a FSI master controller.

FSI master drivers register one of these structs to provide
device-specific of the standard operations: read/write/term/break and
link control.

Includes changes from Edward A. James <eajames-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org> & Jeremy Kerr
<jk-mnsaURCQ41sdnm+yROfE0A@public.gmane.org>.

Signed-off-by: Jeremy Kerr <jk-mnsaURCQ41sdnm+yROfE0A@public.gmane.org>
Signed-off-by: Chris Bostic <cbostic-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
Signed-off-by: Joel Stanley <joel-U3u1mxZcP9KHXe+LvDLADg@public.gmane.org>
---
 drivers/fsi/fsi-core.c   | 35 +++++++++++++++++++++++++++++++++++
 drivers/fsi/fsi-master.h | 41 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 76 insertions(+)
 create mode 100644 drivers/fsi/fsi-master.h

diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c
index 3d55bd5..ca02913 100644
--- a/drivers/fsi/fsi-core.c
+++ b/drivers/fsi/fsi-core.c
@@ -15,8 +15,43 @@
 
 #include <linux/device.h>
 #include <linux/fsi.h>
+#include <linux/idr.h>
 #include <linux/module.h>
 
+#include "fsi-master.h"
+
+static DEFINE_IDA(master_ida);
+
+/* FSI master support */
+int fsi_master_register(struct fsi_master *master)
+{
+	int rc;
+
+	if (!master)
+		return -EINVAL;
+
+	master->idx = ida_simple_get(&master_ida, 0, INT_MAX, GFP_KERNEL);
+	dev_set_name(&master->dev, "fsi%d", master->idx);
+
+	rc = device_register(&master->dev);
+	if (rc)
+		ida_simple_remove(&master_ida, master->idx);
+
+	return rc;
+}
+EXPORT_SYMBOL_GPL(fsi_master_register);
+
+void fsi_master_unregister(struct fsi_master *master)
+{
+	if (master->idx >= 0) {
+		ida_simple_remove(&master_ida, master->idx);
+		master->idx = -1;
+	}
+
+	device_unregister(&master->dev);
+}
+EXPORT_SYMBOL_GPL(fsi_master_unregister);
+
 /* FSI core & Linux bus type definitions */
 
 static int fsi_bus_match(struct device *dev, struct device_driver *drv)
diff --git a/drivers/fsi/fsi-master.h b/drivers/fsi/fsi-master.h
new file mode 100644
index 0000000..7764b00
--- /dev/null
+++ b/drivers/fsi/fsi-master.h
@@ -0,0 +1,41 @@
+/*
+ * FSI master definitions. These comprise the core <--> master interface,
+ * to allow the core to interact with the (hardware-specific) masters.
+ *
+ * Copyright (C) IBM Corporation 2016
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#ifndef DRIVERS_FSI_MASTER_H
+#define DRIVERS_FSI_MASTER_H
+
+#include <linux/device.h>
+
+struct fsi_master {
+	struct device	dev;
+	int		idx;
+	int		n_links;
+	int		flags;
+	int		(*read)(struct fsi_master *, int link, uint8_t id,
+				uint32_t addr, void *val, size_t size);
+	int		(*write)(struct fsi_master *, int link, uint8_t id,
+				uint32_t addr, const void *val, size_t size);
+	int		(*term)(struct fsi_master *, int link, uint8_t id);
+	int		(*send_break)(struct fsi_master *, int link);
+	int		(*link_enable)(struct fsi_master *, int link);
+};
+
+#define dev_to_fsi_master(d) container_of(d, struct fsi_master, dev)
+
+extern int fsi_master_register(struct fsi_master *master);
+extern void fsi_master_unregister(struct fsi_master *master);
+
+#endif /* DRIVERS_FSI_MASTER_H */
-- 
1.8.2.2

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* [PATCH v4 02/23] drivers/fsi: Add slave definition
From: Christopher Bostic @ 2017-03-29 17:43 UTC (permalink / raw)
  To: robh+dt, mark.rutland, linux, rostedt, mingo, gregkh, devicetree,
	linux-arm-kernel
  Cc: Chris Bostic, andrew, alistair, linux-kernel, Jeremy Kerr, benh,
	joel
In-Reply-To: <20170329174340.89109-1-cbostic@linux.vnet.ibm.com>

From: Jeremy Kerr <jk@ozlabs.org>

Add the initial fsi slave device, which is private to the core code.
This will be a child of the master, and parent to endpoint devices.

Signed-off-by: Jeremy Kerr <jk@ozlabs.org>
Signed-off-by: Chris Bostic <cbostic@linux.vnet.ibm.com>
Signed-off-by: Joel Stanley <joel@jms.id.au>
---
 drivers/fsi/fsi-core.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c
index ca02913..2f19509 100644
--- a/drivers/fsi/fsi-core.c
+++ b/drivers/fsi/fsi-core.c
@@ -22,6 +22,16 @@
 
 static DEFINE_IDA(master_ida);
 
+struct fsi_slave {
+	struct device		dev;
+	struct fsi_master	*master;
+	int			id;
+	int			link;
+	uint32_t		size;	/* size of slave address space */
+};
+
+#define to_fsi_slave(d) container_of(d, struct fsi_slave, dev)
+
 /* FSI master support */
 int fsi_master_register(struct fsi_master *master)
 {
-- 
1.8.2.2

^ permalink raw reply related

* [PATCH v4 03/23] drivers/fsi: Add empty master scan
From: Christopher Bostic @ 2017-03-29 17:43 UTC (permalink / raw)
  To: robh+dt, mark.rutland, linux, rostedt, mingo, gregkh, devicetree,
	linux-arm-kernel
  Cc: Chris Bostic, andrew, alistair, linux-kernel, Jeremy Kerr, benh,
	joel
In-Reply-To: <20170329174340.89109-1-cbostic@linux.vnet.ibm.com>

From: Jeremy Kerr <jk@ozlabs.org>

When a new fsi master is added, we will need to scan its links, and
slaves attached to those links. This change introduces a little shell to
iterate the links, which we will populate with the actual slave scan in
a later change.

Signed-off-by: Jeremy Kerr <jk@ozlabs.org>
Signed-off-by: Chris Bostic <cbostic@linux.vnet.ibm.com>
Signed-off-by: Joel Stanley <joel@jms.id.au>
---
 drivers/fsi/fsi-core.c | 25 +++++++++++++++++++++++--
 1 file changed, 23 insertions(+), 2 deletions(-)

diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c
index 2f19509..e90d45d 100644
--- a/drivers/fsi/fsi-core.c
+++ b/drivers/fsi/fsi-core.c
@@ -32,7 +32,25 @@ struct fsi_slave {
 
 #define to_fsi_slave(d) container_of(d, struct fsi_slave, dev)
 
+/* FSI slave support */
+static int fsi_slave_init(struct fsi_master *master, int link, uint8_t id)
+{
+	/* todo: initialise slave device, perform engine scan */
+
+	return -ENODEV;
+}
+
 /* FSI master support */
+static int fsi_master_scan(struct fsi_master *master)
+{
+	int link;
+
+	for (link = 0; link < master->n_links; link++)
+		fsi_slave_init(master, link, 0);
+
+	return 0;
+}
+
 int fsi_master_register(struct fsi_master *master)
 {
 	int rc;
@@ -44,10 +62,13 @@ int fsi_master_register(struct fsi_master *master)
 	dev_set_name(&master->dev, "fsi%d", master->idx);
 
 	rc = device_register(&master->dev);
-	if (rc)
+	if (rc) {
 		ida_simple_remove(&master_ida, master->idx);
+		return rc;
+	}
 
-	return rc;
+	fsi_master_scan(master);
+	return 0;
 }
 EXPORT_SYMBOL_GPL(fsi_master_register);
 
-- 
1.8.2.2

^ permalink raw reply related

* [PATCH v4 04/23] drivers/fsi: Add crc4 helpers
From: Christopher Bostic @ 2017-03-29 17:43 UTC (permalink / raw)
  To: robh+dt, mark.rutland, linux, rostedt, mingo, gregkh, devicetree,
	linux-arm-kernel
  Cc: Chris Bostic, andrew, alistair, linux-kernel, Jeremy Kerr, benh,
	joel
In-Reply-To: <20170329174340.89109-1-cbostic@linux.vnet.ibm.com>

From: Jeremy Kerr <jk@ozlabs.org>

Add some helpers for the crc checks for the slave configuration table.
This works 4-bits-at-a-time, using a simple table approach.

We will need this in the FSI core code, as well as any master
implementations that need to calculate CRCs in software.

We add this to the fsi code (rather than lib/), as we need a specific
polynomial for FSI CRCs.

Signed-off-by: Jeremy Kerr <jk@ozlabs.org>
Signed-off-by: Chris Bostic <cbostic@linux.vnet.ibm.com>
Signed-off-by: Joel Stanley <joel@jms.id.au>
---
 drivers/fsi/fsi-core.c   | 24 ++++++++++++++++++++++++
 drivers/fsi/fsi-master.h | 21 +++++++++++++++++++++
 2 files changed, 45 insertions(+)

diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c
index e90d45d..4bbca95 100644
--- a/drivers/fsi/fsi-core.c
+++ b/drivers/fsi/fsi-core.c
@@ -32,6 +32,30 @@ struct fsi_slave {
 
 #define to_fsi_slave(d) container_of(d, struct fsi_slave, dev)
 
+/* crc helpers */
+static const uint8_t crc4_tab[] = {
+	0x0, 0x7, 0xe, 0x9, 0xb, 0xc, 0x5, 0x2,
+	0x1, 0x6, 0xf, 0x8, 0xa, 0xd, 0x4, 0x3,
+};
+
+uint8_t fsi_crc4(uint8_t c, uint64_t x, int bits)
+{
+	int i;
+
+	/* Align to 4-bits */
+	bits = (bits + 3) & ~0x3;
+
+	/* mask off anything above the top bit */
+	x &= (1ull << bits) - 1;
+
+	/* Calculate crc4 over four-bit nibbles, starting at the MSbit */
+	for (i = bits - 4; i >= 0; i -= 4)
+		c = crc4_tab[c ^ ((x >> i) & 0xf)];
+
+	return c;
+}
+EXPORT_SYMBOL_GPL(fsi_crc4);
+
 /* FSI slave support */
 static int fsi_slave_init(struct fsi_master *master, int link, uint8_t id)
 {
diff --git a/drivers/fsi/fsi-master.h b/drivers/fsi/fsi-master.h
index 7764b00..d6a4885 100644
--- a/drivers/fsi/fsi-master.h
+++ b/drivers/fsi/fsi-master.h
@@ -38,4 +38,25 @@ struct fsi_master {
 extern int fsi_master_register(struct fsi_master *master);
 extern void fsi_master_unregister(struct fsi_master *master);
 
+/**
+ * crc4 helper: Given a starting crc4 state @c, calculate the crc4 vaue of @x,
+ * which is @bits in length. This may be required by master implementations
+ * that do not provide their own hardware checksums.
+ *
+ * The crc4 is performed on 4-bit chunks (which is all we need for FSI
+ * calculations). Typically, we'll want a starting state of 0:
+ *
+ *  c = fsi_crc4(0, msg, len);
+ *
+ * To crc4 a message that includes a single start bit, initialise crc4 state
+ * with:
+ *
+ *  c = fsi_crc4(0, 1, 1);
+ *
+ * Then update with message data:
+ *
+ *  c = fsi_crc4(c, msg, len);
+ */
+uint8_t fsi_crc4(uint8_t c, uint64_t x, int bits);
+
 #endif /* DRIVERS_FSI_MASTER_H */
-- 
1.8.2.2

^ permalink raw reply related

* [PATCH v4 05/23] drivers/fsi: Add slave & master read/write APIs
From: Christopher Bostic @ 2017-03-29 17:43 UTC (permalink / raw)
  To: robh+dt, mark.rutland, linux, rostedt, mingo, gregkh, devicetree,
	linux-arm-kernel
  Cc: Jeremy Kerr, joel, linux-kernel, andrew, alistair, benh,
	Chris Bostic
In-Reply-To: <20170329174340.89109-1-cbostic@linux.vnet.ibm.com>

From: Jeremy Kerr <jk@ozlabs.org>

Introduce functions to perform reads/writes on the slave address space;
these simply pass the request on the slave's master with the correct
link and slave ID.

We implement these on top of similar helpers for the master.

Signed-off-by: Jeremy Kerr <jk@ozlabs.org>
Signed-off-by: Joel Stanley <joel@jms.id.au>
Signed-off-by: Chris Bostic <cbostic@linux.vnet.ibm.com>
---
 drivers/fsi/fsi-core.c | 92 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 92 insertions(+)

diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c
index 4bbca95..32698ed 100644
--- a/drivers/fsi/fsi-core.c
+++ b/drivers/fsi/fsi-core.c
@@ -32,6 +32,11 @@ struct fsi_slave {
 
 #define to_fsi_slave(d) container_of(d, struct fsi_slave, dev)
 
+static int fsi_master_read(struct fsi_master *master, int link,
+		uint8_t slave_id, uint32_t addr, void *val, size_t size);
+static int fsi_master_write(struct fsi_master *master, int link,
+		uint8_t slave_id, uint32_t addr, const void *val, size_t size);
+
 /* crc helpers */
 static const uint8_t crc4_tab[] = {
 	0x0, 0x7, 0xe, 0x9, 0xb, 0xc, 0x5, 0x2,
@@ -57,6 +62,58 @@ uint8_t fsi_crc4(uint8_t c, uint64_t x, int bits)
 EXPORT_SYMBOL_GPL(fsi_crc4);
 
 /* FSI slave support */
+static int fsi_slave_calc_addr(struct fsi_slave *slave, uint32_t *addrp,
+		uint8_t *idp)
+{
+	uint32_t addr = *addrp;
+	uint8_t id = *idp;
+
+	if (addr > slave->size)
+		return -EINVAL;
+
+	/* For 23 bit addressing, we encode the extra two bits in the slave
+	 * id (and the slave's actual ID needs to be 0).
+	 */
+	if (addr > 0x1fffff) {
+		if (slave->id != 0)
+			return -EINVAL;
+		id = (addr >> 21) & 0x3;
+		addr &= 0x1fffff;
+	}
+
+	*addrp = addr;
+	*idp = id;
+	return 0;
+}
+
+static int fsi_slave_read(struct fsi_slave *slave, uint32_t addr,
+			void *val, size_t size)
+{
+	uint8_t id = slave->id;
+	int rc;
+
+	rc = fsi_slave_calc_addr(slave, &addr, &id);
+	if (rc)
+		return rc;
+
+	return fsi_master_read(slave->master, slave->link, id,
+			addr, val, size);
+}
+
+static int fsi_slave_write(struct fsi_slave *slave, uint32_t addr,
+			const void *val, size_t size)
+{
+	uint8_t id = slave->id;
+	int rc;
+
+	rc = fsi_slave_calc_addr(slave, &addr, &id);
+	if (rc)
+		return rc;
+
+	return fsi_master_write(slave->master, slave->link, id,
+			addr, val, size);
+}
+
 static int fsi_slave_init(struct fsi_master *master, int link, uint8_t id)
 {
 	/* todo: initialise slave device, perform engine scan */
@@ -65,6 +122,41 @@ static int fsi_slave_init(struct fsi_master *master, int link, uint8_t id)
 }
 
 /* FSI master support */
+static int fsi_check_access(uint32_t addr, size_t size)
+{
+	if (size != 1 && size != 2 && size != 4)
+		return -EINVAL;
+
+	if ((addr & 0x3) != (size & 0x3))
+		return -EINVAL;
+
+	return 0;
+}
+
+static int fsi_master_read(struct fsi_master *master, int link,
+		uint8_t slave_id, uint32_t addr, void *val, size_t size)
+{
+	int rc;
+
+	rc = fsi_check_access(addr, size);
+	if (rc)
+		return rc;
+
+	return master->read(master, link, slave_id, addr, val, size);
+}
+
+static int fsi_master_write(struct fsi_master *master, int link,
+		uint8_t slave_id, uint32_t addr, const void *val, size_t size)
+{
+	int rc;
+
+	rc = fsi_check_access(addr, size);
+	if (rc)
+		return rc;
+
+	return master->write(master, link, slave_id, addr, val, size);
+}
+
 static int fsi_master_scan(struct fsi_master *master)
 {
 	int link;
-- 
1.8.2.2

^ permalink raw reply related


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