Devicetree
 help / color / mirror / Atom feed
* Re: [PATCH 2/5] drivers: mmc: sunxi: limit A64 MMC2 to 8K DMA buffer
From: Maxime Ripard @ 2017-01-10 14:06 UTC (permalink / raw)
  To: André Przywara
  Cc: Rob Herring, Ulf Hansson, Chen-Yu Tsai, Hans De Goede,
	Icenowy Zheng, Mark Rutland, devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-mmc-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-sunxi-/JYPxA39Uh5TLH3MbocFFw,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <87b26848-4f86-f2d2-3f82-db0937c572e2-5wv7dgnIgG8@public.gmane.org>

[-- Attachment #1: Type: text/plain, Size: 2610 bytes --]

On Thu, Jan 05, 2017 at 11:33:28PM +0000, André Przywara wrote:
> On 05/01/17 17:57, Maxime Ripard wrote:
> > Hi Rob,
> > 
> > On Wed, Jan 04, 2017 at 08:07:50AM -0600, Rob Herring wrote:
> >> On Mon, Jan 02, 2017 at 11:03:43PM +0000, Andre Przywara wrote:
> >>> From: Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> >>>
> >>> Unlike the A64 user manual reports, the third MMC controller on the
> >>> A64 (and the only one capable of 8-bit HS400 eMMC transfers) has a
> >>> DMA buffer size limit of 8KB (much like the very old Allwinner SoCs).
> >>> This does not affect the other two controllers, so introduce a new
> >>> DT compatible string to let the driver use different settings for that
> >>> particular device. This will also help to enable the high-speed transfer
> >>> modes of that controller later.
> >>>
> >>> Signed-off-by: Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> >>> Signed-off-by: Andre Przywara <andre.przywara-5wv7dgnIgG8@public.gmane.org>
> >>> ---
> >>>  Documentation/devicetree/bindings/mmc/sunxi-mmc.txt | 1 +
> >>>  drivers/mmc/host/sunxi-mmc.c                        | 7 +++++++
> >>>  2 files changed, 8 insertions(+)
> >>
> >> Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> > 
> > Some kind of a digression on this: we have three MMC controllers on
> > this SoC. Like this patch shows, the third one is clearly different,
> > and supports both more modes, a wider bus, and specific quirks. We
> > need a new compatible for this one, everything's perfect.
> > 
> > However, the other two are mostly the same, but seems to need
> > different tuning parameters to get more performances out of the
> > controller (but this is unclear yet). How do we usually deal with
> > that?
> 
> I guess you wanted to hear Rob's opinion ;-), but "get more performance"
> sounds like we add one (or more) properties to tune those values.
> If I get this right, it works with default values, but is sub-optimal?

That would be my understanding too, at least, it works in a decent way
without fiddling with those parameters.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

-- 
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.

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

^ permalink raw reply

* Re: [PATCH 1/5] pinctrl: core: Use delayed work for hogs
From: Geert Uytterhoeven @ 2017-01-10 14:08 UTC (permalink / raw)
  To: Tony Lindgren
  Cc: Linus Walleij, Haojian Zhuang, Masahiro Yamada, Grygorii Strashko,
	Nishanth Menon, linux-gpio@vger.kernel.org,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-omap@vger.kernel.org, Linux-Renesas
In-Reply-To: <20161227172003.6517-2-tony@atomide.com>

Hi Tony,

On Tue, Dec 27, 2016 at 6:19 PM, Tony Lindgren <tony@atomide.com> wrote:
> Having the pin control framework call pin controller functions
> before it's probe has finished is not nice as the pin controller
> device driver does not yet have struct pinctrl_dev handle.
>
> Let's fix this issue by adding deferred work for late init. This is
> needed to be able to add pinctrl generic helper functions that expect
> to know struct pinctrl_dev handle. Note that we now need to call
> create_pinctrl() directly as we don't want to add the pin controller
> to the list of controllers until the hogs are claimed. We also need
> to pass the pinctrl_dev to the device tree parser functions as they
> otherwise won't find the right controller at this point.
>
> Signed-off-by: Tony Lindgren <tony@atomide.com>

I believe this patch causes a regression on r8a7740/armadillo, where the
pin controller is also a GPIO controller, and lcd0 needs a hog
(cfr. arch/arm/boot/dts/r8a7740-armadillo800eva.dts):

-GPIO line 176 (lcd0) hogged as output/high
-sh-pfc e6050000.pfc: r8a7740_pfc handling gpio 0 -> 211
+gpiochip_add_data: GPIOs 0..211 (r8a7740_pfc) failed to register
+sh-pfc e6050000.pfc: failed to init GPIO chip, ignoring...
 sh-pfc e6050000.pfc: r8a7740_pfc support registered

Hence all drivers using GPIOs fail to initialize because their GPIOs never
become available.

Adding debug prints to the failure paths shows that the call to
of_pinctrl_get() in of_gpiochip_add_pin_range() fails with -EPROBE_DEFER.
Adding a debug print to the top of gpiochip_add_data() makes the problem go
away, presumably because it introduces a delay that allows the delayed work
to kick in...

Jon's fix ("pinctrl: core: Fix panic when pinctrl devices with hogs are
unregistered") doesn't help, as it affects unregistration only.

> --- a/drivers/pinctrl/core.c
> +++ b/drivers/pinctrl/core.c

> @@ -1800,32 +1847,10 @@ struct pinctrl_dev *pinctrl_register(struct pinctrl_desc *pctldesc,
>                 goto out_err;
>         }
>
> -       mutex_lock(&pinctrldev_list_mutex);
> -       list_add_tail(&pctldev->node, &pinctrldev_list);
> -       mutex_unlock(&pinctrldev_list_mutex);
> -
> -       pctldev->p = pinctrl_get(pctldev->dev);
> -
> -       if (!IS_ERR(pctldev->p)) {
> -               pctldev->hog_default =
> -                       pinctrl_lookup_state(pctldev->p, PINCTRL_STATE_DEFAULT);
> -               if (IS_ERR(pctldev->hog_default)) {
> -                       dev_dbg(dev, "failed to lookup the default state\n");
> -               } else {
> -                       if (pinctrl_select_state(pctldev->p,
> -                                               pctldev->hog_default))
> -                               dev_err(dev,
> -                                       "failed to select default state\n");
> -               }
> -
> -               pctldev->hog_sleep =
> -                       pinctrl_lookup_state(pctldev->p,
> -                                                   PINCTRL_STATE_SLEEP);
> -               if (IS_ERR(pctldev->hog_sleep))
> -                       dev_dbg(dev, "failed to lookup the sleep state\n");
> -       }
> -
> -       pinctrl_init_device_debugfs(pctldev);
> +       if (pinctrl_dt_has_hogs(pctldev))

Changing the above line to "if (0 && pinctrl_dt_has_hogs(pctldev))"
fixes the issue for me.

> +               schedule_delayed_work(&pctldev->late_init, 0);
> +       else
> +               pinctrl_late_init(&pctldev->late_init.work);
>
>         return pctldev;

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* Re: [PATCH v6 0/2] Support for Axentia TSE-850
From: Alexandre Belloni @ 2017-01-10 15:17 UTC (permalink / raw)
  To: Peter Rosin
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Mark Rutland,
	Russell King, Nicolas Ferre, Jean-Christophe Plagniol-Villard,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1484054314-6244-1-git-send-email-peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>

On 10/01/2017 at 14:18:32 +0100, Peter Rosin wrote :
> Hi!
> 
> changes v5 -> v6
> - drop the &main clock-frequency
> - add some text to the commit messages
> - make config additions modules
> 
> changes v4 -> v5
> - comment from Rob about the memory node made me look closer and
>   the memory size is actually updated by the bootloader, and that
>   hid the fact that the entry was always faulty. This version
>   specifies the correct memory size from the start, which is 64MB.
> - ack from Rob
> 
> changes v3 -> v4
> - rename files arch/arm/boot/dts/axentia-* to .../at91-*
> - remove bootargs from at91-tse850-3.dts
> - depend on the atmel ssc to register as a sound dai by itself
> - bump copyright years
> 
> changes v2 -> v3
> - document the new compatible strings prefixed with "axentia,".
> 
> changes v1 -> v2
> - squash the fixup into the correct patch, sorry for the noise.
> 
> After finally having all essintial drivers upstreamed I would
> like to have the dts and the defconfig also upstreamed.
> 
> The atmel-ssc/sound-dai change depends on a change that has been
> sitting in the ASoC tree since mid-december, and I have been waiting
> for it to hit linux-next before sending this, but it seems to take
> longer than I anticipated. So, since I do not want this to in
> turn miss the next merge window because of that wait I therefore
> request that this is taken now even though it doesn't really work
> w/o the ASoC "topic/atmel" branch as of 2016-12-15 [1]. It of course
> builds cleanly even w/o those ASoC changes. That effectively means
> that noone besides me should notice the inconsistency (I currently
> have all affected devices under my control).
> 
> Cheers,
> peda
> 
> [1] http://git.kernel.org/cgit/linux/kernel/git/broonie/sound.git/log/?h=topic/atmel
> 
> Peter Rosin (2):
>   ARM: dts: at91: add devicetree for the Axentia TSE-850
>   ARM: sama5_defconfig: add support for the Axentia TSE-850 board
> 
>  Documentation/devicetree/bindings/arm/axentia.txt |  19 ++
>  MAINTAINERS                                       |   8 +
>  arch/arm/boot/dts/Makefile                        |   1 +
>  arch/arm/boot/dts/at91-linea.dtsi                 |  49 ++++
>  arch/arm/boot/dts/at91-tse850-3.dts               | 274 ++++++++++++++++++++++
>  arch/arm/configs/sama5_defconfig                  |   7 +-
>  6 files changed, 357 insertions(+), 1 deletion(-)
>  create mode 100644 Documentation/devicetree/bindings/arm/axentia.txt
>  create mode 100644 arch/arm/boot/dts/at91-linea.dtsi
>  create mode 100644 arch/arm/boot/dts/at91-tse850-3.dts
> 

Both applied, thanks!

-- 
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
--
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 08/11] dmaengine: cppi41: Implement the glue for da8xx
From: Alexandre Bailon @ 2017-01-10 15:22 UTC (permalink / raw)
  To: Sekhar Nori, Grygorii Strashko, vinod.koul-ral2JQCrhuEAvxtiuMwx3w
  Cc: dmaengine-u79uwXL29TY76Z2rM5mHXA,
	linux-usb-u79uwXL29TY76Z2rM5mHXA, khilman-rdvid1DuHRBWk0Htik3J/w,
	ptitiano-rdvid1DuHRBWk0Htik3J/w, tony-4v6yS6AI5VpBDgjK7y7TUQ,
	linux-omap-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	b-liu-l0cyMroinI0
In-Reply-To: <53a40635-2652-64fc-b20d-1cd6d813eacb-l0cyMroinI0@public.gmane.org>

On 01/10/2017 11:05 AM, Sekhar Nori wrote:
> On Tuesday 10 January 2017 03:08 PM, Alexandre Bailon wrote:
>> On 01/09/2017 07:08 PM, Grygorii Strashko wrote:
>>>
>>>
>>> On 01/09/2017 10:06 AM, Alexandre Bailon wrote:
>>>> The da8xx has a cppi41 dma controller.
>>>> This is add the glue layer required to make it work on da8xx,
>>>> as well some changes in driver (e.g to manage clock).
>>>>
>>>> Signed-off-by: Alexandre Bailon <abailon-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
>>>> ---
>>>>  drivers/dma/cppi41.c | 95 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>>  1 file changed, 95 insertions(+)
>>>>
>>>> diff --git a/drivers/dma/cppi41.c b/drivers/dma/cppi41.c
>>>> index 939398e..4318e53 100644
>>>> --- a/drivers/dma/cppi41.c
>>>> +++ b/drivers/dma/cppi41.c
>>>> @@ -1,3 +1,4 @@
>>>> +#include <linux/clk.h>
>>>>  #include <linux/delay.h>
>>>>  #include <linux/dmaengine.h>
>>>>  #include <linux/dma-mapping.h>
>>>> @@ -86,10 +87,19 @@
>>>>  
>>>>  #define USBSS_IRQ_PD_COMP	(1 <<  2)
>>>>  
>>>> +/* USB DA8XX */
>>>> +#define DA8XX_INTR_SRC_MASKED	0x38
>>>> +#define DA8XX_END_OF_INTR	0x3c
>>>> +
>>>> +#define DA8XX_QMGR_PENDING_MASK	(0xf << 24)
>>>> +
>>>> +
>>>> +
>>>>  /* Packet Descriptor */
>>>>  #define PD2_ZERO_LENGTH		(1 << 19)
>>>>  
>>>>  #define AM335X_CPPI41		0
>>>> +#define DA8XX_CPPI41		1
>>>>  
>>>>  struct cppi41_channel {
>>>>  	struct dma_chan chan;
>>>> @@ -158,6 +168,9 @@ struct cppi41_dd {
>>>>  
>>>>  	/* context for suspend/resume */
>>>>  	unsigned int dma_tdfdq;
>>>> +
>>>> +	/* da8xx clock */
>>>> +	struct clk *clk;
>>>>  };
>>>>  
>>>>  static struct chan_queues am335x_usb_queues_tx[] = {
>>>> @@ -232,6 +245,20 @@ static const struct chan_queues am335x_usb_queues_rx[] = {
>>>>  	[29] = { .submit = 30, .complete = 155},
>>>>  };
>>>>  
>>>> +static const struct chan_queues da8xx_usb_queues_tx[] = {
>>>> +	[0] = { .submit =  16, .complete = 24},
>>>> +	[1] = { .submit =  18, .complete = 24},
>>>> +	[2] = { .submit =  20, .complete = 24},
>>>> +	[3] = { .submit =  22, .complete = 24},
>>>> +};
>>>> +
>>>> +static const struct chan_queues da8xx_usb_queues_rx[] = {
>>>> +	[0] = { .submit =  1, .complete = 26},
>>>> +	[1] = { .submit =  3, .complete = 26},
>>>> +	[2] = { .submit =  5, .complete = 26},
>>>> +	[3] = { .submit =  7, .complete = 26},
>>>> +};
>>>> +
>>>>  struct cppi_glue_infos {
>>>>  	irqreturn_t (*isr)(int irq, void *data);
>>>>  	const struct chan_queues *queues_rx;
>>>> @@ -366,6 +393,26 @@ static irqreturn_t am335x_cppi41_irq(int irq, void *data)
>>>>  	return cppi41_irq(cdd);
>>>>  }
>>>>  
>>>> +static irqreturn_t da8xx_cppi41_irq(int irq, void *data)
>>>> +{
>>>> +	struct cppi41_dd *cdd = data;
>>>> +	u32 status;
>>>> +	u32 usbss_status;
>>>> +
>>>> +	status = cppi_readl(cdd->qmgr_mem + QMGR_PEND(0));
>>>> +	if (status & DA8XX_QMGR_PENDING_MASK)
>>>> +		cppi41_irq(cdd);
>>>> +	else
>>>> +		return IRQ_NONE;
>>>> +
>>>> +	/* Re-assert IRQ if there no usb core interrupts pending */
>>>> +	usbss_status = cppi_readl(cdd->usbss_mem + DA8XX_INTR_SRC_MASKED);
>>>> +	if (!usbss_status)
>>>> +		cppi_writel(0, cdd->usbss_mem + DA8XX_END_OF_INTR);
>>>> +
>>>> +	return IRQ_HANDLED;
>>>> +}
>>>> +
>>>>  static dma_cookie_t cppi41_tx_submit(struct dma_async_tx_descriptor *tx)
>>>>  {
>>>>  	dma_cookie_t cookie;
>>>> @@ -972,8 +1019,19 @@ static const struct cppi_glue_infos am335x_usb_infos = {
>>>>  	.platform = AM335X_CPPI41,
>>>>  };
>>>>  
>>>> +static const struct cppi_glue_infos da8xx_usb_infos = {
>>>> +	.isr = da8xx_cppi41_irq,
>>>> +	.queues_rx = da8xx_usb_queues_rx,
>>>> +	.queues_tx = da8xx_usb_queues_tx,
>>>> +	.td_queue = { .submit = 31, .complete = 0 },
>>>> +	.first_completion_queue = 24,
>>>> +	.qmgr_num_pend = 2,
>>>> +	.platform = DA8XX_CPPI41,
>>>> +};
>>>> +
>>>>  static const struct of_device_id cppi41_dma_ids[] = {
>>>>  	{ .compatible = "ti,am3359-cppi41", .data = &am335x_usb_infos},
>>>> +	{ .compatible = "ti,da8xx-cppi41", .data = &da8xx_usb_infos},
>>>>  	{},
>>>>  };
>>>>  MODULE_DEVICE_TABLE(of, cppi41_dma_ids);
>>>> @@ -995,6 +1053,13 @@ static int is_am335x_cppi41(struct device *dev)
>>>>  	return cdd->platform == AM335X_CPPI41;
>>>>  }
>>>>  
>>>> +static int is_da8xx_cppi41(struct device *dev)
>>>> +{
>>>> +	struct cppi41_dd *cdd = dev_get_drvdata(dev);
>>>> +
>>>> +	return cdd->platform == DA8XX_CPPI41;
>>>> +}
>>>> +
>>>>  #define CPPI41_DMA_BUSWIDTHS	(BIT(DMA_SLAVE_BUSWIDTH_1_BYTE) | \
>>>>  				BIT(DMA_SLAVE_BUSWIDTH_2_BYTES) | \
>>>>  				BIT(DMA_SLAVE_BUSWIDTH_3_BYTES) | \
>>>> @@ -1058,6 +1123,21 @@ static int cppi41_dma_probe(struct platform_device *pdev)
>>>>  	cdd->first_completion_queue = glue_info->first_completion_queue;
>>>>  	cdd->platform = glue_info->platform;
>>>>  
>>>> +	if (is_da8xx_cppi41(dev)) {
>>>> +		cdd->clk = devm_clk_get(&pdev->dev, "usb20");
>>>> +		ret = PTR_ERR_OR_ZERO(cdd->clk);
>>>> +		if (ret) {
>>>> +			dev_err(&pdev->dev, "failed to get clock\n");
>>>> +			goto err_clk_en;
>>>> +		}
>>>> +
>>>> +		ret = clk_prepare_enable(cdd->clk);
>>>> +		if (ret) {
>>>> +			dev_err(dev, "failed to enable clock\n");
>>>> +			goto err_clk_en;
>>>> +		}
>>>> +	}
>>>
>>> if this is functional clock then why not to use ./arch/arm/mach-davinci/pm_domain.c ?
>>> wouldn't it work for use if you will just rename "usb20" -> "fck" -
>>> so PM runtime should manage this clock for you?
>> As is, I don't think it will work.
>> The usb20 is shared by the cppi41 and the usb otg.
>> So, if we rename "usb20" to "fck", clk_get() won't be able to find the
>> clock.
>> But may be adding "usb20" to "con_ids" in
>> arch/arm/mach-davinci/pm_domain.c could work.
>> But I think it will require some changes in da8xx musb driver.
>> I will take look.
> 
> On DA8xx, CPPI 4.1 DMAengine is not an independent system resource, but
> embedded within the USB 2.0 controller. So, I think all that is needed
> is for MUSB DA8xx glue to trigger probe of CPPI 4.1 dmaengine driver
> when it is ready. I am not sure all this DA850-specific clock handling
> is really necessary.
Actually, we have a circular dependency.
USB core tries to get DMA channels during the probe, which fails because
CPPI 4.1 driver is not ready.
But it will never be ready because the USB clock must be enabled before
DMA driver probe, what will not happen because USB driver have disabled
the clock when probe failed.

Someone in the office suggested me to use the component API,
that could help me to probe the DMA from the USB probe.

Another way to workaround the dependency would be to do defer the
function calls that access to hardware to avoid to control clock from
DMA driver.
> 
> Even in DT, the CPPI 4.1 node should be a child node of USB 2.0 node.
I agree, it should a child but it would require some changes in CPPI 4.1
driver. But except to have a better hardware description, I don't see
any benefit to do it.
> 
> Thanks,
> sekhar
> 
Thanks,
Alexandre
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" 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 0/2] iio: distance: srf08: add IIO driver for us ranger
From: Andreas Klinger @ 2017-01-10 15:26 UTC (permalink / raw)
  To: jic23, knaack.h, lars, pmeerw, linux-iio, linux-kernel, ktsai,
	wsa, robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak,
	trivial, mranostay, linux-i2c, devicetree
  Cc: ak

This patch series adds IIO driver support for srf08 ultrasonic ranger
devices.

The first patch add a trivial device tree binding for the device together
with a new vendor devantech.

The second patch is the IIO driver which in turn is using I2C to talk to
the device.

Documentation about the sensor can be found here:

http://www.robot-electronics.co.uk/htm/srf08tech.html

Andreas Klinger (2):
  iio: distance: srf08: add trivial DT binding
  iio: distance: srf08: add IIO driver for srf08

 .../devicetree/bindings/i2c/trivial-devices.txt    |   1 +
 .../devicetree/bindings/vendor-prefixes.txt        |   1 +
 drivers/iio/proximity/Kconfig                      |  15 +
 drivers/iio/proximity/Makefile                     |   1 +
 drivers/iio/proximity/srf08.c                      | 345 +++++++++++++++++++++
 5 files changed, 363 insertions(+)
 create mode 100644 drivers/iio/proximity/srf08.c

-- 
2.1.4

^ permalink raw reply

* [PATCH 1/2] iio: distance: srf08: add trivial DT binding
From: Andreas Klinger @ 2017-01-10 15:27 UTC (permalink / raw)
  To: jic23, knaack.h, lars, pmeerw, linux-iio, linux-kernel, ktsai,
	wsa, robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak,
	trivial, mranostay, linux-i2c, devicetree
  Cc: ak

Add DT binding for devantech,srf08
Add vendor devantech to vendor list

Signed-off-by: Andreas Klinger <ak@it-klinger.de>
---
 Documentation/devicetree/bindings/i2c/trivial-devices.txt | 1 +
 Documentation/devicetree/bindings/vendor-prefixes.txt     | 1 +
 2 files changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/i2c/trivial-devices.txt b/Documentation/devicetree/bindings/i2c/trivial-devices.txt
index 539874490492..86c6930c3c91 100644
--- a/Documentation/devicetree/bindings/i2c/trivial-devices.txt
+++ b/Documentation/devicetree/bindings/i2c/trivial-devices.txt
@@ -36,6 +36,7 @@ dallas,ds1775		Tiny Digital Thermometer and Thermostat
 dallas,ds3232		Extremely Accurate I²C RTC with Integrated Crystal and SRAM
 dallas,ds4510		CPU Supervisor with Nonvolatile Memory and Programmable I/O
 dallas,ds75		Digital Thermometer and Thermostat
+devantech,srf08		Devantech SRF08 ultrasonic ranger
 dlg,da9053		DA9053: flexible system level PMIC with multicore support
 dlg,da9063		DA9063: system PMIC for quad-core application processors
 epson,rx8010		I2C-BUS INTERFACE REAL TIME CLOCK MODULE
diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
index 4696bb5c2198..80325e602403 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.txt
+++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
@@ -65,6 +65,7 @@ dallas	Maxim Integrated Products (formerly Dallas Semiconductor)
 davicom	DAVICOM Semiconductor, Inc.
 delta	Delta Electronics, Inc.
 denx	Denx Software Engineering
+devantech	Devantech, Ltd.
 digi	Digi International Inc.
 digilent	Diglent, Inc.
 dlg	Dialog Semiconductor
-- 
2.1.4

^ permalink raw reply related

* [PATCH 2/2] iio: distance: srf08: add IIO driver for us ranger
From: Andreas Klinger @ 2017-01-10 15:28 UTC (permalink / raw)
  To: jic23-DgEjT+Ai2ygdnm+yROfE0A, knaack.h-Mmb7MZpHnFY,
	lars-Qo5EllUWu/uELgA04lAiVw, pmeerw-jW+XmwGofnusTnJN9+BGXg,
	linux-iio-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, ktsai-GubuWUlQtMwciDkP5Hr2oA,
	wsa-z923LK4zBo2bacvFa/9K2g, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	pawel.moll-5wv7dgnIgG8, mark.rutland-5wv7dgnIgG8,
	ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg,
	galak-sgV2jX0FEOL9JmXXK+q4OQ, trivial-DgEjT+Ai2ygdnm+yROfE0A,
	mranostay-Re5JQEeQqe8AvxtiuMwx3w,
	linux-i2c-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA
  Cc: ak-n176/SwNRljddJNmlsFzeA

This is the IIO driver for devantech srf08 ultrasonic ranger which can be
used to measure the distances to an object.

The sensor supports I2C with some registers.

Supported Features include:
- read the distance in ranging mode in centimeters
- output of the driver is directly the read value
- together with the scale the driver delivers the distance in meters
- only the first echo of the nearest object is delivered
- set max gain register; userspace enters analogue value
- set range registers; userspace enters range in millimeters in 43 mm steps

Features not supported by this driver:
- ranging mode in inches or in microseconds
- ANN mode
- change I2C address through this driver
- light sensor

The driver was added in the directory "proximity" of the iio subsystem
in absence of another directory named "distance".
There is also a new submenu "distance"

Signed-off-by: Andreas Klinger <ak-n176/SwNRljddJNmlsFzeA@public.gmane.org>
---
 drivers/iio/proximity/Kconfig  |  15 ++
 drivers/iio/proximity/Makefile |   1 +
 drivers/iio/proximity/srf08.c  | 345 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 361 insertions(+)
 create mode 100644 drivers/iio/proximity/srf08.c

diff --git a/drivers/iio/proximity/Kconfig b/drivers/iio/proximity/Kconfig
index ef4c73db5b53..7b10a137702b 100644
--- a/drivers/iio/proximity/Kconfig
+++ b/drivers/iio/proximity/Kconfig
@@ -46,3 +46,18 @@ config SX9500
 	  module will be called sx9500.
 
 endmenu
+
+menu "Distance sensors"
+
+config SRF08
+	tristate "Devantech SRF08 ultrasonic ranger sensor"
+	depends on I2C
+	help
+	  Say Y here to build a driver for Devantech SRF08 ultrasonic
+	  ranger sensor. This driver can be used to measure the distance
+	  of objects.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called srf08.
+
+endmenu
diff --git a/drivers/iio/proximity/Makefile b/drivers/iio/proximity/Makefile
index 9aadd9a8ee99..14d8fe4e5cae 100644
--- a/drivers/iio/proximity/Makefile
+++ b/drivers/iio/proximity/Makefile
@@ -6,3 +6,4 @@
 obj-$(CONFIG_AS3935)		+= as3935.o
 obj-$(CONFIG_LIDAR_LITE_V2)	+= pulsedlight-lidar-lite-v2.o
 obj-$(CONFIG_SX9500)		+= sx9500.o
+obj-$(CONFIG_SRF08)		+= srf08.o
diff --git a/drivers/iio/proximity/srf08.c b/drivers/iio/proximity/srf08.c
new file mode 100644
index 000000000000..a888a230b419
--- /dev/null
+++ b/drivers/iio/proximity/srf08.c
@@ -0,0 +1,345 @@
+/*
+ * srf08.c - Support for Devantech SRF08 ultrasonic ranger
+ *
+ * Copyright (c) 2016 Andreas Klinger <ak-n176/SwNRljddJNmlsFzeA@public.gmane.org>
+ *
+ * This file is subject to the terms and conditions of version 2 of
+ * the GNU General Public License.  See the file COPYING in the main
+ * directory of this archive for more details.
+ *
+ * For details about the device see:
+ * http://www.robot-electronics.co.uk/htm/srf08tech.html
+ */
+
+#include <linux/err.h>
+#include <linux/i2c.h>
+#include <linux/delay.h>
+#include <linux/module.h>
+#include <linux/bitops.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+
+/* registers of SRF08 device */
+#define SRF08_WRITE_COMMAND	0x00	/* Command Register */
+#define SRF08_WRITE_MAX_GAIN	0x01	/* Max Gain Register: 0 .. 31 */
+#define SRF08_WRITE_RANGE	0x02	/* Range Register: 0 .. 255 */
+#define SRF08_READ_SW_REVISION	0x00	/* Software Revision */
+#define SRF08_READ_LIGHT	0x01	/* Light Sensor during last echo */
+#define SRF08_READ_ECHO_1_HIGH	0x02	/* range of first echo received */
+#define SRF08_READ_ECHO_1_LOW	0x03	/* range of first echo received */
+
+#define SRF08_CMD_RANGING_CM	0x51	/* Ranging Mode - Result in cm */
+
+struct srf08_data {
+	struct i2c_client	*client;
+	int			gain;			/* max gain */
+	int			range_mm;		/* range in mm */
+	struct mutex		lock;
+};
+
+static const int srf08_gain[] = {
+	 94,  97, 100, 103, 107, 110, 114, 118,
+	123, 128, 133, 139, 145, 152, 159, 168,
+	177, 187, 199, 212, 227, 245, 265, 288,
+	317, 352, 395, 450, 524, 626, 777, 1025 };
+
+static int srf08_read_ranging(struct srf08_data *data)
+{
+	struct i2c_client *client = data->client;
+	char cmd[2];
+	int ret, i;
+
+	mutex_lock(&data->lock);
+
+	ret = i2c_smbus_write_byte_data(data->client,
+			SRF08_WRITE_COMMAND, SRF08_CMD_RANGING_CM);
+	if (ret < 0) {
+		dev_err(&client->dev, "write command - err: %d\n", ret);
+		mutex_unlock(&data->lock);
+		return ret;
+	}
+
+	/*
+	 * normally after 65 ms the device should have the read value
+	 * we round it up to 100 ms
+	 */
+	for (i = 0; i < 5; i++) {
+
+		ret = i2c_smbus_read_byte_data(data->client,
+						SRF08_READ_SW_REVISION);
+
+		/* check if a valid version number is read */
+		if ((ret < 255) && (ret > 0))
+			break;
+		msleep(20);
+	}
+
+	if ((ret >= 255) || (ret <= 0)) {
+		dev_err(&client->dev, "device not ready\n");
+		mutex_unlock(&data->lock);
+		return -EIO;
+	}
+
+	memset(cmd, 0, sizeof(cmd));
+
+	for (i = 0; i < 2; i++) {
+		ret = i2c_smbus_read_byte_data(data->client,
+						SRF08_READ_ECHO_1_HIGH + i);
+		cmd[i] = ret;
+	}
+
+	mutex_unlock(&data->lock);
+
+	return (cmd[0] << 8 | cmd[1]);
+}
+
+static int srf08_read_raw(struct iio_dev *indio_dev,
+			    struct iio_chan_spec const *channel, int *val,
+			    int *val2, long mask)
+{
+	struct srf08_data *data = iio_priv(indio_dev);
+	s32 ret;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		if (channel->type == IIO_DISTANCE) {
+			ret = srf08_read_ranging(data);
+			*val = ret;
+		} else {
+			break;
+		}
+		return IIO_VAL_INT;
+	case IIO_CHAN_INFO_SCALE:
+		if (channel->type == IIO_DISTANCE) {
+			/* 1 LSB is 1 cm */
+			*val = 0;
+			*val2 = 10000;
+		} else {
+			break;
+		}
+		return IIO_VAL_INT_PLUS_MICRO;
+	default:
+		break;
+	}
+
+	return -EINVAL;
+}
+
+static ssize_t srf08_show_range_mm_available(struct device *dev,
+				struct device_attribute *attr, char *buf)
+{
+	int i, len = 0;
+
+	for (i = 0; i < 256; i++)
+		len += sprintf(buf + len, "%d ", (i + 1) * 43);
+
+	len += sprintf(buf + len, "\n");
+
+	return len;
+}
+
+static IIO_DEVICE_ATTR(range_mm_available, S_IRUGO,
+				srf08_show_range_mm_available, NULL, 0);
+
+static ssize_t srf08_show_range_mm(struct device *dev,
+				struct device_attribute *attr, char *buf)
+{
+	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
+	struct srf08_data *data = iio_priv(indio_dev);
+	int len;
+
+	len = sprintf(buf, "%d\n", data->range_mm);
+
+	return len;
+}
+
+static ssize_t srf08_write_range_mm(struct device *dev,
+					struct device_attribute *attr,
+					const char *buf, size_t len)
+{
+	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
+	struct srf08_data *data = iio_priv(indio_dev);
+	struct i2c_client *client = data->client;
+	int ret, val;
+	int regval = -1;
+	int mod;
+
+	ret = kstrtoint(buf, 10, &val);
+	if (ret)
+		return ret;
+
+	regval = val / 43 - 1;
+	mod = val % 43;
+
+	if (mod || (regval < 0) || (regval > 255))
+		return -EINVAL;
+
+	mutex_lock(&data->lock);
+
+	ret = i2c_smbus_write_byte_data(data->client,
+						SRF08_WRITE_RANGE, regval);
+	if (ret < 0) {
+		dev_err(&client->dev, "write_range - err: %d\n", ret);
+		mutex_unlock(&data->lock);
+		return ret;
+	}
+
+	data->range_mm = val;
+
+	mutex_unlock(&data->lock);
+
+	return len;
+}
+
+static IIO_DEVICE_ATTR(range_mm, S_IRUGO | S_IWUSR,
+			srf08_show_range_mm, srf08_write_range_mm, 0);
+
+static ssize_t srf08_show_gain_available(struct device *dev,
+				struct device_attribute *attr, char *buf)
+{
+	int i, len = 0;
+
+	for (i = 0; i < ARRAY_SIZE(srf08_gain); i++)
+		len += sprintf(buf + len, "%d ", srf08_gain[i]);
+
+	len += sprintf(buf + len, "\n");
+
+	return len;
+}
+
+static IIO_DEVICE_ATTR(gain_available, S_IRUGO,
+				srf08_show_gain_available, NULL, 0);
+
+static ssize_t srf08_show_gain(struct device *dev,
+				struct device_attribute *attr, char *buf)
+{
+	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
+	struct srf08_data *data = iio_priv(indio_dev);
+	int len;
+
+	len = sprintf(buf, "%d\n", data->gain);
+
+	return len;
+}
+
+static ssize_t srf08_write_gain(struct device *dev,
+						struct device_attribute *attr,
+						const char *buf, size_t len)
+{
+	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
+	struct srf08_data *data = iio_priv(indio_dev);
+	struct i2c_client *client = data->client;
+	int ret, val, i;
+	int regval = -1;
+
+	ret = kstrtoint(buf, 10, &val);
+	if (ret)
+		return ret;
+
+	for (i = 0; i < ARRAY_SIZE(srf08_gain); i++)
+		if (val == srf08_gain[i])
+			regval = i;
+
+	if (regval == -1)
+		return -EINVAL;
+
+	mutex_lock(&data->lock);
+
+	ret = i2c_smbus_write_byte_data(data->client,
+						SRF08_WRITE_MAX_GAIN, regval);
+	if (ret < 0) {
+		dev_err(&client->dev, "write_gain - err: %d\n", ret);
+		mutex_unlock(&data->lock);
+		return ret;
+	}
+
+	data->gain = val;
+
+	mutex_unlock(&data->lock);
+
+	return len;
+}
+
+static IIO_DEVICE_ATTR(gain, S_IRUGO | S_IWUSR,
+					srf08_show_gain, srf08_write_gain, 0);
+
+static struct attribute *srf08_attributes[] = {
+	&iio_dev_attr_range_mm.dev_attr.attr,
+	&iio_dev_attr_range_mm_available.dev_attr.attr,
+	&iio_dev_attr_gain.dev_attr.attr,
+	&iio_dev_attr_gain_available.dev_attr.attr,
+	NULL,
+};
+
+static const struct attribute_group srf08_attribute_group = {
+	.attrs = srf08_attributes,
+};
+
+static const struct iio_chan_spec srf08_channels[] = {
+	{
+		.type = IIO_DISTANCE,
+		.info_mask_separate =
+				BIT(IIO_CHAN_INFO_RAW) |
+				BIT(IIO_CHAN_INFO_SCALE),
+	},
+};
+
+static const struct iio_info srf08_info = {
+	.read_raw = srf08_read_raw,
+	.attrs = &srf08_attribute_group,
+	.driver_module = THIS_MODULE,
+};
+
+static int srf08_probe(struct i2c_client *client,
+					 const struct i2c_device_id *id)
+{
+	struct iio_dev *indio_dev;
+	struct srf08_data *data;
+
+	if (!i2c_check_functionality(client->adapter,
+					I2C_FUNC_SMBUS_READ_BYTE_DATA |
+					I2C_FUNC_SMBUS_WRITE_BYTE_DATA))
+		return -ENODEV;
+
+	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	data = iio_priv(indio_dev);
+	i2c_set_clientdata(client, indio_dev);
+	data->client = client;
+
+	/* set default values of device */
+	data->gain = 1025;
+	data->range_mm = 11008;
+
+	indio_dev->name = dev_name(&client->dev);
+	indio_dev->dev.parent = &client->dev;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+	indio_dev->info = &srf08_info;
+	indio_dev->channels = srf08_channels;
+	indio_dev->num_channels = ARRAY_SIZE(srf08_channels);
+
+	mutex_init(&data->lock);
+
+	return devm_iio_device_register(&client->dev, indio_dev);
+}
+
+static const struct i2c_device_id srf08_id[] = {
+	{ "srf08", 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, srf08_id);
+
+static struct i2c_driver srf08_driver = {
+	.driver = {
+		.name	= "srf08",
+	},
+	.probe = srf08_probe,
+	.id_table = srf08_id,
+};
+module_i2c_driver(srf08_driver);
+
+MODULE_AUTHOR("Andreas Klinger <ak-n176/SwNRljddJNmlsFzeA@public.gmane.org>");
+MODULE_DESCRIPTION("Devantech SRF08 ultrasonic ranger driver");
+MODULE_LICENSE("GPL");
-- 
2.1.4

--
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

* Re: [PATCH 1/5] pinctrl: core: Use delayed work for hogs
From: Tony Lindgren @ 2017-01-10 15:30 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Linus Walleij, Haojian Zhuang, Masahiro Yamada, Grygorii Strashko,
	Nishanth Menon, linux-gpio@vger.kernel.org,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-omap@vger.kernel.org, Linux-Renesas
In-Reply-To: <CAMuHMdVLbNXx23-wgLHcvktyHxWf6t75ggU3rM5A-DhtU9hn5w@mail.gmail.com>

* Geert Uytterhoeven <geert@linux-m68k.org> [170110 06:09]:
> Hi Tony,
> 
> On Tue, Dec 27, 2016 at 6:19 PM, Tony Lindgren <tony@atomide.com> wrote:
> > Having the pin control framework call pin controller functions
> > before it's probe has finished is not nice as the pin controller
> > device driver does not yet have struct pinctrl_dev handle.
> >
> > Let's fix this issue by adding deferred work for late init. This is
> > needed to be able to add pinctrl generic helper functions that expect
> > to know struct pinctrl_dev handle. Note that we now need to call
> > create_pinctrl() directly as we don't want to add the pin controller
> > to the list of controllers until the hogs are claimed. We also need
> > to pass the pinctrl_dev to the device tree parser functions as they
> > otherwise won't find the right controller at this point.
> >
> > Signed-off-by: Tony Lindgren <tony@atomide.com>
> 
> I believe this patch causes a regression on r8a7740/armadillo, where the
> pin controller is also a GPIO controller, and lcd0 needs a hog
> (cfr. arch/arm/boot/dts/r8a7740-armadillo800eva.dts):
> 
> -GPIO line 176 (lcd0) hogged as output/high
> -sh-pfc e6050000.pfc: r8a7740_pfc handling gpio 0 -> 211
> +gpiochip_add_data: GPIOs 0..211 (r8a7740_pfc) failed to register
> +sh-pfc e6050000.pfc: failed to init GPIO chip, ignoring...
>  sh-pfc e6050000.pfc: r8a7740_pfc support registered
> 
> Hence all drivers using GPIOs fail to initialize because their GPIOs never
> become available.
> 
> Adding debug prints to the failure paths shows that the call to
> of_pinctrl_get() in of_gpiochip_add_pin_range() fails with -EPROBE_DEFER.
> Adding a debug print to the top of gpiochip_add_data() makes the problem go
> away, presumably because it introduces a delay that allows the delayed work
> to kick in...

OK. What if we added also an optional pinctrl function that the pin
controller driver could call to initialize hogs? Then the pin controller
driver could call it during or after probe as needed. That is after
there's a valid struct pinctrl_dev handle.

> Jon's fix ("pinctrl: core: Fix panic when pinctrl devices with hogs are
> unregistered") doesn't help, as it affects unregistration only.
> 
> > --- a/drivers/pinctrl/core.c
> > +++ b/drivers/pinctrl/core.c
> 
> > @@ -1800,32 +1847,10 @@ struct pinctrl_dev *pinctrl_register(struct pinctrl_desc *pctldesc,
> >                 goto out_err;
> >         }
> >
> > -       mutex_lock(&pinctrldev_list_mutex);
> > -       list_add_tail(&pctldev->node, &pinctrldev_list);
> > -       mutex_unlock(&pinctrldev_list_mutex);
> > -
> > -       pctldev->p = pinctrl_get(pctldev->dev);
> > -
> > -       if (!IS_ERR(pctldev->p)) {
> > -               pctldev->hog_default =
> > -                       pinctrl_lookup_state(pctldev->p, PINCTRL_STATE_DEFAULT);
> > -               if (IS_ERR(pctldev->hog_default)) {
> > -                       dev_dbg(dev, "failed to lookup the default state\n");
> > -               } else {
> > -                       if (pinctrl_select_state(pctldev->p,
> > -                                               pctldev->hog_default))
> > -                               dev_err(dev,
> > -                                       "failed to select default state\n");
> > -               }
> > -
> > -               pctldev->hog_sleep =
> > -                       pinctrl_lookup_state(pctldev->p,
> > -                                                   PINCTRL_STATE_SLEEP);
> > -               if (IS_ERR(pctldev->hog_sleep))
> > -                       dev_dbg(dev, "failed to lookup the sleep state\n");
> > -       }
> > -
> > -       pinctrl_init_device_debugfs(pctldev);
> > +       if (pinctrl_dt_has_hogs(pctldev))
> 
> Changing the above line to "if (0 && pinctrl_dt_has_hogs(pctldev))"
> fixes the issue for me.
> 
> > +               schedule_delayed_work(&pctldev->late_init, 0);
> > +       else
> > +               pinctrl_late_init(&pctldev->late_init.work);
> >
> >         return pctldev;

We could also pass some flag if should always call pinctrl_late_init()
directly. But that does not remove the problem of struct pinctrl_dev handle
being uninitialized when the pin controller driver functionas are called.

Regards,

Tony

^ permalink raw reply

* Re: [PATCH v2 3/5] ARM: davinci_all_defconfig: enable iio and ADS7950
From: David Lechner @ 2017-01-10 15:43 UTC (permalink / raw)
  To: Sekhar Nori
  Cc: Kevin Hilman, Rob Herring, Mark Rutland,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <cb9455bc-2658-9826-823b-2aa845237fed-l0cyMroinI0@public.gmane.org>

On 01/09/2017 06:29 AM, Sekhar Nori wrote:
> On Friday 06 January 2017 10:03 AM, David Lechner wrote:
>> This enables the iio subsystem and the TI ADS7950 driver. This is used by
>> LEGO MINDSTORMS EV3, which has an ADS7957 chip.
>
> Can you add your sign-off?
>
>> ---
>>
>> The CONFIG_TI_ADS7950 driver is currently in iio/testing, so some coordination
>> may be needed before picking up this patch.
>>
>>  arch/arm/configs/davinci_all_defconfig | 7 +++++++
>>  1 file changed, 7 insertions(+)
>>
>> diff --git a/arch/arm/configs/davinci_all_defconfig b/arch/arm/configs/davinci_all_defconfig
>> index 2b1967a..a899876 100644
>> --- a/arch/arm/configs/davinci_all_defconfig
>> +++ b/arch/arm/configs/davinci_all_defconfig
>> @@ -200,6 +200,13 @@ CONFIG_TI_EDMA=y
>>  CONFIG_MEMORY=y
>>  CONFIG_TI_AEMIF=m
>>  CONFIG_DA8XX_DDRCTL=y
>> +CONFIG_IIO=m
>> +CONFIG_IIO_BUFFER_CB=m
>> +CONFIG_IIO_SW_DEVICE=m
>> +CONFIG_IIO_SW_TRIGGER=m
>
>> +CONFIG_TI_ADS7950=m
>
> Can you separate this from rest of the patch. I would like to enable
> this option only after I can find the symbol in linux-next.

Will resend without CONFIG_TI_ADS7950

>
>> +CONFIG_IIO_HRTIMER_TRIGGER=m
>> +CONFIG_IIO_SYSFS_TRIGGER=m
>
> Need CONFIG_IIO_TRIGGER=y also for these two options to take effect.

CONFIG_IIO_TRIGGER is selected by IIO_TRIGGERED_BUFFER [=m] && IIO [=m] 
&& IIO_BUFFER [=y], so save_defconfig does not pick it up.


>
> Thanks,
> Sekhar
>

--
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] Documentation: dt-bindings: Add binding documentation for TI clkctrl clocks
From: Tony Lindgren @ 2017-01-10 15:44 UTC (permalink / raw)
  To: Michael Turquette, Stephen Boyd, Tero Kristo
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-clk-u79uwXL29TY76Z2rM5mHXA,
	linux-omap-u79uwXL29TY76Z2rM5mHXA, Paul Walmsley, Rob Herring
In-Reply-To: <20170109234226.9449-1-tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>

* Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> [170109 15:43]:
> Texas Instruments omap variant SoCs starting with omap4 have a clkctrl
> clock controller instance for each interconnect target module. The clkctrl
> controls functional and interface clocks for the module.
> 
> The clkctrl clocks are currently handled by arch/arm/mach-omap2 hwmod code.
> With this binding and a related clock device driver we can start moving the
> clkctrl clock handling to live in drivers/clk/ti.
> 
> For hardware reference, see omap4430 TRM "Table 3-1312. L4PER_CM2 Registers
> Mapping Summary" for example. It show one instance of a clkctrl clock
> controller with multiple clkctrl registers.
> 
> Note that this binding allows keeping the clockdomain related parts out of
> drivers/clock. The CLKCTCTRL and DYNAMICDEP registers can be handled by
> using a separate driver in drivers/soc/ti and genpd. If the clockdomain
> driver needs to know it's clocks, we can just set the the clkctrl device
> instances to be children of the related clockdomain device.
> 
> On omap4 CM_L3INIT_USB_HOST_HS_CLKCTRL on omap5 has eight OPTFCLKEN bits.
> So we need to shift the clock index to avoid index conflict for the clock
> consumer binding with the next clkctrl offset on omap4.
> 
> Cc: Paul Walmsley <paul-DWxLp4Yu+b8AvxtiuMwx3w@public.gmane.org>
> Cc: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Signed-off-by: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
> ---
> 
> So here's what I was able to come up for the clkctr binding based on
> all we've discussed so far. Can you guys please take a look and see
> if it looks OK to you before we do the device driver?
> 
> Also, does anybody have better suggestions for addressing the optional
> clocks in each clkctrl register?

The other option that might be worth considering is to make use of the
#clock-cells property. Then the index of any optional clock could be passed
in the second cell.

The third cell could be used to set the modulemode for the clock (software
controlled or hardware controlled) instead of using a custom property
at the clock controllel level.

In that case clock consume usage would look like the following using
#clock-cells = <3>:

#define OMAP4_CLKCTRL_OFFSET		0x20
#define MODULEMODE_HWCTRL		1
#define MODULEMODE_SWCTRL		2

#define OMAP_CLKCTRL_INDEX(offset)	((offset) - OMAP4_CLKCTRL_OFFSET)

#define OMAP4_GPTIMER10_CLKTRL		OMAP_CLKCTRL_INDEX(0x28)
#define OMAP4_GPTIMER11_CLKTRL		OMAP_CLKCTRL_INDEX(0x30)
#define OMAP4_GPTIMER2_CLKTRL		OMAP_CLKCTRL_INDEX(0x38)
...
#define OMAP4_GPIO2_CLKCTRL		OMAP_CLKCTRL_INDEX(0x60)
...

&gpio2 {
	clocks = <&cm_l4per_clkctrl OMAP4_GPIO2_CLKCTRL 0 MODULEMODE_HWCTRL
		  &cm_l4per_clkctrl OMAP4_GPIO2_CLKCTRL_DBCLK 1 0>;
};

Regards,

Tony


>  .../devicetree/bindings/clock/ti-clkctrl.txt       | 56 ++++++++++++++++++++++
>  1 file changed, 56 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/clock/ti-clkctrl.txt
> 
> diff --git a/Documentation/devicetree/bindings/clock/ti-clkctrl.txt b/Documentation/devicetree/bindings/clock/ti-clkctrl.txt
> new file mode 100644
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/ti-clkctrl.txt
> @@ -0,0 +1,56 @@
> +Texas Instruments clkctrl clock binding
> +
> +Texas Instruments SoCs can have a clkctrl clock controller for each
> +interconnect target module. The clkctrl clock controller manages functional
> +and interface clocks for each module. Each clkctrl controller can also
> +gate one or more optional functional clocks for a module. The clkctrl
> +clock controller is typical for omap3 and later variants.
> +
> +The clock consumers can specify the index of the clkctrl clock using
> +the hardware offset from the clkctrl instance register space. The optional
> +functional clocks can be specified by clkctrl hardware offset plus the
> +index of the optional clock. Please see the Linux clock framework binding
> +at Documentation/devicetree/bindings/clock/clock-bindings.txt.
> +
> +Required properties :
> +- compatible : shall be "ti,clkctrl"
> +- #clock-cells : shall contain 1
> +
> +Optional properties :
> +- "ti,modulemode-auto" : list of clkctrl offsets using automatic gating
> +
> +Example: Clock controller node:
> +
> +&cm_l4per {
> +	cm_l4per_clkctrl: clk@20 {
> +		compatible = "ti,clkctrl";
> +		reg = <0x20 0x1b0>;
> +		#clock-cells = 1;
> +		ti,modulemode-auto = <OMAP4_GPIO2_CLKCTRL>;
> +	};
> +};
> +
> +Example: Preprocessor helper macros in dt-bindings/ti-clkctrl.h
> +
> +#define OMAP4_CLKCTRL_OFFSET		0x20
> +
> +#define OMAP_CLKCTRL_INDEX(offset)		\
> +	(((offset) - OMAP4_CLKCTRL_OFFSET) << 8)
> +
> +#define OMAP_CLKCTRL_OPT_INDEX(offset, optclk)	\
> +	(OMAP_CLKCTRL_INDEX(offset) + (optclk))
> +
> +#define OMAP4_GPTIMER10_CLKTRL		OMAP_CLKCTRL_INDEX(0x28)
> +#define OMAP4_GPTIMER11_CLKTRL		OMAP_CLKCTRL_INDEX(0x30)
> +#define OMAP4_GPTIMER2_CLKTRL		OMAP_CLKCTRL_INDEX(0x38)
> +...
> +#define OMAP4_GPIO2_CLKCTRL		OMAP_CLKCTRL_INDEX(0x60)
> +#define OMAP4_GPIO2_CLKCTRL_DBCLK	OMAP_CLKCTRL_OPT_INDEX(0x60, 1)
> +...
> +
> +Example: Clock consumer node for GPIO2:
> +
> +&gpio2 {
> +       clocks = <&cm_l4per_clkctrl OMAP4_GPIO2_CLKCTRL
> +		 &cm_l4per_clkctrl OMAP4_GPIO2_CLKCTRL_DBCLK>;
> +};
> -- 
> 2.11.0
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
--
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 08/11] dmaengine: cppi41: Implement the glue for da8xx
From: Tony Lindgren @ 2017-01-10 15:49 UTC (permalink / raw)
  To: Alexandre Bailon
  Cc: Sekhar Nori, Grygorii Strashko, vinod.koul-ral2JQCrhuEAvxtiuMwx3w,
	dmaengine-u79uwXL29TY76Z2rM5mHXA,
	linux-usb-u79uwXL29TY76Z2rM5mHXA, khilman-rdvid1DuHRBWk0Htik3J/w,
	ptitiano-rdvid1DuHRBWk0Htik3J/w,
	linux-omap-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	b-liu-l0cyMroinI0
In-Reply-To: <43b9585d-a22f-a2ff-15d4-5d878bd1586a-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>

* Alexandre Bailon <abailon-rdvid1DuHRBWk0Htik3J/w@public.gmane.org> [170110 07:23]:
> On 01/10/2017 11:05 AM, Sekhar Nori wrote:
> > On DA8xx, CPPI 4.1 DMAengine is not an independent system resource, but
> > embedded within the USB 2.0 controller. So, I think all that is needed
> > is for MUSB DA8xx glue to trigger probe of CPPI 4.1 dmaengine driver
> > when it is ready. I am not sure all this DA850-specific clock handling
> > is really necessary.
> Actually, we have a circular dependency.
> USB core tries to get DMA channels during the probe, which fails because
> CPPI 4.1 driver is not ready.
> But it will never be ready because the USB clock must be enabled before
> DMA driver probe, what will not happen because USB driver have disabled
> the clock when probe failed.
> 
> Someone in the office suggested me to use the component API,
> that could help me to probe the DMA from the USB probe.
> 
> Another way to workaround the dependency would be to do defer the
> function calls that access to hardware to avoid to control clock from
> DMA driver.

Or you really have some wrapper IP block around musb and cppi41 just
like am335x has.

See drivers/usb/musb/musb_am335x.c and compatible properties for
"ti,am33xx-usb" and it's children in am33xx.dtsi.

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" 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/3] ARM: dts: da850: Add the cppi41 dma node
From: David Lechner @ 2017-01-10 16:15 UTC (permalink / raw)
  To: Sergei Shtylyov, Alexandre Bailon, nsekhar-l0cyMroinI0,
	khilman-rdvid1DuHRBWk0Htik3J/w
  Cc: vinod.koul-ral2JQCrhuEAvxtiuMwx3w,
	dmaengine-u79uwXL29TY76Z2rM5mHXA,
	linux-usb-u79uwXL29TY76Z2rM5mHXA, ptitiano-rdvid1DuHRBWk0Htik3J/w,
	tony-4v6yS6AI5VpBDgjK7y7TUQ, linux-omap-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	b-liu-l0cyMroinI0
In-Reply-To: <811adb6b-00e4-bbc4-d6f5-7bf3ba85ab63-M4DtvfQ/ZS1MRgGoP+s0PdBPR1lH4CV8@public.gmane.org>

On 01/09/2017 12:26 PM, Sergei Shtylyov wrote:
> Hello.
>
> On 01/09/2017 07:24 PM, Alexandre Bailon wrote:
>
>> This adds the device tree node for the cppi41 dma
>> used by the usb otg controller present in the da850 family of SoC's.
>>
>> Signed-off-by: Alexandre Bailon <abailon-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
>> ---
>>  arch/arm/boot/dts/da850.dtsi | 16 ++++++++++++++++
>>  1 file changed, 16 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
>> index 104155d..d6b406a 100644
>> --- a/arch/arm/boot/dts/da850.dtsi
>> +++ b/arch/arm/boot/dts/da850.dtsi
>> @@ -403,6 +403,22 @@
>>              phy-names = "usb-phy";
>>              status = "disabled";
>>          };
>> +        cppi41dma: dma-controller@201000 {
>> +            compatible = "ti,da8xx-cppi41";
>> +            reg =  <0x200000 0x1000
>
>    I don't remember any DA8xx glue regs having to do with the CPPI 4.1...
>
>> +                0x201000 0x1000
>> +                0x202000 0x1000
>> +                0x204000 0x4000>;
>> +            reg-names = "glue", "controller",
>> +                    "scheduler", "queuemgr";
>> +            interrupts = <58>;
>> +            interrupt-names = "glue";
>> +            #dma-cells = <2>;
>> +            #dma-channels = <4>;
>> +            #dma-requests = <256>;
>> +            status = "disabled";
>
>    Why disabled? It doesn't use any external pins...

Why enable it if musb node is not also enabled?

>
> [...]
>
> MBR, Sergei
>

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" 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] of: alloc anywhere from memblock if range not specified
From: Leonard Crestez @ 2017-01-10 16:16 UTC (permalink / raw)
  To: Vinayak Menon
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	frowand.list-Re5JQEeQqe8AvxtiuMwx3w,
	grant.likely-QSEj5FYQhm4dnm+yROfE0A,
	m.szyprowski-Sze3O3UU22JBDgjK7y7TUQ,
	vinayakm.list-Re5JQEeQqe8AvxtiuMwx3w, Octavian Purdila
In-Reply-To: <1456148744-7583-1-git-send-email-vinmenon-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>

Hello,

I have some trouble with this patch.

It seems the intention is to allow CMA to be placed in highmem. If the 
CMA area is larger than highmem and no alloc-ranges is specified (just a 
size) it is possible to end up allocating a area that spans from 
multiple zones. This later breaks checks in cma_activate_area and makes 
most dma allocations fail.

Am I missing something or this a bug?

On Mon, Feb 22, 2016 at 3:45 PM, Vinayak Menon <vinmenon-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> 
wrote:
>
> early_init_dt_alloc_reserved_memory_arch passes end as 0 to
> __memblock_alloc_base, when limits are not specified. But
> __memblock_alloc_base takes end value of 0 as MEMBLOCK_ALLOC_ACCESSIBLE
> and limits the end to memblock.current_limit. This results in regions
> never being placed in HIGHMEM area, for e.g. CMA.
> Let __memblock_alloc_base allocate from anywhere in memory if limits are
> not specified.
>
> Acked-by: Marek Szyprowski <m.szyprowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> Signed-off-by: Vinayak Menon <vinmenon-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
> ---
>  drivers/of/of_reserved_mem.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
> index 1a3556a..ed01c01 100644
> --- a/drivers/of/of_reserved_mem.c
> +++ b/drivers/of/of_reserved_mem.c
> @@ -32,11 +32,13 @@ int __init __weak early_init_dt_alloc_reserved_memory_arch(phys_addr_t size,
>         phys_addr_t align, phys_addr_t start, phys_addr_t end, bool nomap,
>         phys_addr_t *res_base)
>  {
> +       phys_addr_t base;
>         /*
>          * We use __memblock_alloc_base() because memblock_alloc_base()
>          * panic()s on allocation failure.
>          */
> -       phys_addr_t base = __memblock_alloc_base(size, align, end);
> +       end = !end ? MEMBLOCK_ALLOC_ANYWHERE : end;
> +       base = __memblock_alloc_base(size, align, end);
>         if (!base)
>                 return -ENOMEM;
>
> --
> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
> member of the Code Aurora Forum, hosted by The Linux Foundation
>
--
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/3] ARM: at91: flush the L2 cache before entering cpu idle
From: Alexandre Belloni @ 2017-01-10 16:18 UTC (permalink / raw)
  To: Wenyou Yang
  Cc: Russell King, Nicolas Ferre, Rob Herring, Mark Rutland,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Wenyou Yang,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <20170106065947.30631-2-wenyou.yang-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>

I though a bit more about it, and I don't really like the new compatible
string. I don't feel this should be necessary.

What about the following:

diff --git a/arch/arm/mach-at91/pm.c b/arch/arm/mach-at91/pm.c
index b4332b727e9c..0333aca63e44 100644
--- a/arch/arm/mach-at91/pm.c
+++ b/arch/arm/mach-at91/pm.c
@@ -53,6 +53,7 @@ extern void at91_pinctrl_gpio_resume(void);
 static struct {
 	unsigned long uhp_udp_mask;
 	int memctrl;
+	bool has_l2_cache;
 } at91_pm_data;
 
 void __iomem *at91_ramc_base[2];
@@ -267,6 +268,11 @@ static void at91_ddr_standby(void)
 	u32 lpr0, lpr1 = 0;
 	u32 saved_lpr0, saved_lpr1 = 0;
 
+	if (at91_pm_data.has_l2_cache) {
+		flush_cache_all();
+		outer_disable();
+	}
+
 	if (at91_ramc_base[1]) {
 		saved_lpr1 = at91_ramc_read(1, AT91_DDRSDRC_LPR);
 		lpr1 = saved_lpr1 & ~AT91_DDRSDRC_LPCB;
@@ -287,6 +293,9 @@ static void at91_ddr_standby(void)
 	at91_ramc_write(0, AT91_DDRSDRC_LPR, saved_lpr0);
 	if (at91_ramc_base[1])
 		at91_ramc_write(1, AT91_DDRSDRC_LPR, saved_lpr1);
+
+	if (at91_pm_data.has_l2_cache)
+		outer_resume();
 }
 
 /* We manage both DDRAM/SDRAM controllers, we need more than one value
 * to
@@ -353,6 +362,11 @@ static __init void at91_dt_ramc(void)
 		return;
 	}
 
+	np = of_find_compatible_node(NULL, NULL, "arm,pl310-cache");
+	if (np)
+		at91_pm_data.has_l2_cache = true;
+	of_node_put(np);
+
 	at91_pm_set_standby(standby);
 }


This has the following benefits:
 - everybody will have the fix, regardless of whether the dtb is updated
 - has_l2_cache can be used later in at91_pm_suspend instead of calling
   it unconditionnaly (I'll send a patch)


On 06/01/2017 at 14:59:45 +0800, Wenyou Yang wrote :
> For the SoCs such as SAMA5D2 and SAMA5D4 which have L2 cache,
> flush the L2 cache first before entering the cpu idle.
> 
> Signed-off-by: Wenyou Yang <wenyou.yang-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>
> ---
> 
>  arch/arm/mach-at91/pm.c       | 19 +++++++++++++++++++
>  drivers/memory/atmel-sdramc.c |  1 +
>  2 files changed, 20 insertions(+)
> 
> diff --git a/arch/arm/mach-at91/pm.c b/arch/arm/mach-at91/pm.c
> index b4332b727e9c..1a60dede1a01 100644
> --- a/arch/arm/mach-at91/pm.c
> +++ b/arch/arm/mach-at91/pm.c
> @@ -289,6 +289,24 @@ static void at91_ddr_standby(void)
>  		at91_ramc_write(1, AT91_DDRSDRC_LPR, saved_lpr1);
>  }
>  
> +static void at91_ddr_cache_standby(void)
> +{
> +	u32 saved_lpr;
> +
> +	flush_cache_all();
> +	outer_disable();
> +
> +	saved_lpr = at91_ramc_read(0, AT91_DDRSDRC_LPR);
> +	at91_ramc_write(0, AT91_DDRSDRC_LPR, (saved_lpr &
> +			(~AT91_DDRSDRC_LPCB)) | AT91_DDRSDRC_LPCB_SELF_REFRESH);
> +
> +	cpu_do_idle();
> +
> +	at91_ramc_write(0, AT91_DDRSDRC_LPR, saved_lpr);
> +
> +	outer_resume();
> +}
> +
>  /* We manage both DDRAM/SDRAM controllers, we need more than one value to
>   * remember.
>   */
> @@ -324,6 +342,7 @@ static const struct of_device_id const ramc_ids[] __initconst = {
>  	{ .compatible = "atmel,at91sam9260-sdramc", .data = at91sam9_sdram_standby },
>  	{ .compatible = "atmel,at91sam9g45-ddramc", .data = at91_ddr_standby },
>  	{ .compatible = "atmel,sama5d3-ddramc", .data = at91_ddr_standby },
> +	{ .compatible = "atmel,sama5d4-ddramc", .data = at91_ddr_cache_standby },
>  	{ /*sentinel*/ }
>  };
>  
> diff --git a/drivers/memory/atmel-sdramc.c b/drivers/memory/atmel-sdramc.c
> index b418b39af180..7e5c5c6c1348 100644
> --- a/drivers/memory/atmel-sdramc.c
> +++ b/drivers/memory/atmel-sdramc.c
> @@ -48,6 +48,7 @@ static const struct of_device_id atmel_ramc_of_match[] = {
>  	{ .compatible = "atmel,at91sam9260-sdramc", .data = &at91rm9200_caps, },
>  	{ .compatible = "atmel,at91sam9g45-ddramc", .data = &at91sam9g45_caps, },
>  	{ .compatible = "atmel,sama5d3-ddramc", .data = &sama5d3_caps, },
> +	{ .compatible = "atmel,sama5d4-ddramc", .data = &sama5d3_caps, },
>  	{},
>  };
>  
> -- 
> 2.11.0
> 

-- 
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
--
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 1/2] ARM: dts: imx6: Specify 'anatop-enable-bit' where appropriate
From: Andrey Smirnov @ 2017-01-10 16:30 UTC (permalink / raw)
  To: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
  Cc: Andrey Smirnov, yurovsky-Re5JQEeQqe8AvxtiuMwx3w, Shawn Guo,
	Sascha Hauer, Fabio Estevam, Rob Herring, Mark Rutland,
	Russell King, devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

ENABLE_LINREG bit is implemented by 3P0, 1P1 and 2P5 regulators on
i.MX6. This property is present in similar code in Fresscale BSP and
made its way upstream in imx6ul.dtsi, so this patch adds this property
to the rest of i.MX6 family for completness.

Cc: yurovsky-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org
Cc: Shawn Guo <shawnguo-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Cc: Sascha Hauer <kernel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
Cc: Fabio Estevam <fabio.estevam-3arQi8VN3Tc@public.gmane.org>
Cc: Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Cc: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
Cc: Russell King <linux-I+IVW8TIWO2tmTQ+vhA3Yw@public.gmane.org>
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Signed-off-by: Andrey Smirnov <andrew.smirnov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 arch/arm/boot/dts/imx6qdl.dtsi | 3 +++
 arch/arm/boot/dts/imx6sl.dtsi  | 3 +++
 arch/arm/boot/dts/imx6sx.dtsi  | 3 +++
 3 files changed, 9 insertions(+)

diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi
index 89b834f..9702e18 100644
--- a/arch/arm/boot/dts/imx6qdl.dtsi
+++ b/arch/arm/boot/dts/imx6qdl.dtsi
@@ -635,6 +635,7 @@
 					anatop-min-bit-val = <4>;
 					anatop-min-voltage = <800000>;
 					anatop-max-voltage = <1375000>;
+					anatop-enable-bit = <0>;
 				};
 
 				regulator-3p0 {
@@ -649,6 +650,7 @@
 					anatop-min-bit-val = <0>;
 					anatop-min-voltage = <2625000>;
 					anatop-max-voltage = <3400000>;
+					anatop-enable-bit = <0>;
 				};
 
 				regulator-2p5 {
@@ -663,6 +665,7 @@
 					anatop-min-bit-val = <0>;
 					anatop-min-voltage = <2000000>;
 					anatop-max-voltage = <2750000>;
+					anatop-enable-bit = <0>;
 				};
 
 				reg_arm: regulator-vddcore {
diff --git a/arch/arm/boot/dts/imx6sl.dtsi b/arch/arm/boot/dts/imx6sl.dtsi
index 19cbd87..a478a06f 100644
--- a/arch/arm/boot/dts/imx6sl.dtsi
+++ b/arch/arm/boot/dts/imx6sl.dtsi
@@ -522,6 +522,7 @@
 					anatop-min-bit-val = <4>;
 					anatop-min-voltage = <800000>;
 					anatop-max-voltage = <1375000>;
+					anatop-enable-bit = <0>;
 				};
 
 				regulator-3p0 {
@@ -536,6 +537,7 @@
 					anatop-min-bit-val = <0>;
 					anatop-min-voltage = <2625000>;
 					anatop-max-voltage = <3400000>;
+					anatop-enable-bit = <0>;
 				};
 
 				regulator-2p5 {
@@ -550,6 +552,7 @@
 					anatop-min-bit-val = <0>;
 					anatop-min-voltage = <2100000>;
 					anatop-max-voltage = <2850000>;
+					anatop-enable-bit = <0>;
 				};
 
 				reg_arm: regulator-vddcore {
diff --git a/arch/arm/boot/dts/imx6sx.dtsi b/arch/arm/boot/dts/imx6sx.dtsi
index 10f3330..3de3cca 100644
--- a/arch/arm/boot/dts/imx6sx.dtsi
+++ b/arch/arm/boot/dts/imx6sx.dtsi
@@ -578,6 +578,7 @@
 					anatop-min-bit-val = <4>;
 					anatop-min-voltage = <800000>;
 					anatop-max-voltage = <1375000>;
+					anatop-enable-bit = <0>;
 				};
 
 				regulator-3p0 {
@@ -592,6 +593,7 @@
 					anatop-min-bit-val = <0>;
 					anatop-min-voltage = <2625000>;
 					anatop-max-voltage = <3400000>;
+					anatop-enable-bit = <0>;
 				};
 
 				regulator-2p5 {
@@ -606,6 +608,7 @@
 					anatop-min-bit-val = <0>;
 					anatop-min-voltage = <2100000>;
 					anatop-max-voltage = <2875000>;
+					anatop-enable-bit = <0>;
 				};
 
 				reg_arm: regulator-vddcore {
-- 
2.9.3

--
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] regulator: anatop: Add support for "anatop-enable-bit"
From: Andrey Smirnov @ 2017-01-10 16:30 UTC (permalink / raw)
  To: linux-kernel
  Cc: Andrey Smirnov, yurovsky, Liam Girdwood, Mark Brown, Rob Herring,
	Mark Rutland, Shawn Guo, devicetree
In-Reply-To: <20170110163015.22444-1-andrew.smirnov@gmail.com>

Add code to support support for "anatop-enable-bit" device-tree
property. This property translates to LINREG_ENABLE bit in real hardware
and is present on 1p1, 2p5 and 3p0 regulators on i.MX6 and 1p0d regulator
on i.MX7.

Cc: yurovsky@gmail.com
Cc: Liam Girdwood <lgirdwood@gmail.com>
Cc: Mark Brown <broonie@kernel.org>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Shawn Guo <shawnguo@kernel.org>
Cc: devicetree@vger.kernel.org
Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---

Note: "anatop-enable-bit" has already found its way into upstream tree
before this patch (probably not on purpose). See imx6ul.dtsi and
imx7s.dtsi for concrete examples

 .../devicetree/bindings/regulator/anatop-regulator.txt       |  1 +
 drivers/regulator/anatop-regulator.c                         | 12 ++++++++++++
 2 files changed, 13 insertions(+)

diff --git a/Documentation/devicetree/bindings/regulator/anatop-regulator.txt b/Documentation/devicetree/bindings/regulator/anatop-regulator.txt
index 37c4ea0..1d58c8c 100644
--- a/Documentation/devicetree/bindings/regulator/anatop-regulator.txt
+++ b/Documentation/devicetree/bindings/regulator/anatop-regulator.txt
@@ -14,6 +14,7 @@ Optional properties:
 - anatop-delay-bit-shift: Bit shift for the step time register
 - anatop-delay-bit-width: Number of bits used in the step time register
 - vin-supply: The supply for this regulator
+- anatop-enable-bit: Regulator enable bit offset
 
 Any property defined as part of the core regulator
 binding, defined in regulator.txt, can also be used.
diff --git a/drivers/regulator/anatop-regulator.c b/drivers/regulator/anatop-regulator.c
index 3a6d029..b041f27 100644
--- a/drivers/regulator/anatop-regulator.c
+++ b/drivers/regulator/anatop-regulator.c
@@ -301,7 +301,19 @@ static int anatop_regulator_probe(struct platform_device *pdev)
 			return -EINVAL;
 		}
 	} else {
+		u32 enable_bit;
+
 		rdesc->ops = &anatop_rops;
+
+		if (!of_property_read_u32(np, "anatop-enable-bit",
+					  &enable_bit)) {
+			anatop_rops.enable  = regulator_enable_regmap;
+			anatop_rops.disable = regulator_disable_regmap;
+			anatop_rops.is_enabled = regulator_is_enabled_regmap;
+
+			rdesc->enable_reg = sreg->control_reg;
+			rdesc->enable_mask = BIT(enable_bit);
+		}
 	}
 
 	/* register regulator */
-- 
2.9.3

^ permalink raw reply related

* [PATCH 2/2] ARM: dts: imx7s: Adjust anatop-enable-bit for 'reg_1p0d'
From: Andrey Smirnov @ 2017-01-10 16:30 UTC (permalink / raw)
  To: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
  Cc: Andrey Smirnov, yurovsky-Re5JQEeQqe8AvxtiuMwx3w, Shawn Guo,
	Sascha Hauer, Fabio Estevam, Rob Herring, Mark Rutland,
	Russell King, devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20170110163015.22444-1-andrew.smirnov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

In PMU_REG_1P0Dn ENABLE_LINREG is bit 0. Bit 31 is called OVERRIDE and
it serves the funtion of granting permission to GPC IP block to alter
various other bit-fields of the register. The reason why this property,
that trickeld here from Freescale BSP, is set up like that is because in
the code it came from it is used in conjunction with a notifier handler
for REGULATOR_EVENT_PRE_DO_ENABLE and REGULATOR_EVENT_PRE_DO_DISABLE
events (not found in upstream kernel) that triggers GPC to start
manipulating aforementioned other bitfields.

Since:
	a) none of the aforementioned machinery is implemented by
	   upstream
	b) using 'anatop-enable-bit' in that capacity is a bit of a
	   semantic stretch

simplify the situation by setting the value of 'anatop-enable-bit' to
point to ENABLE_LINREG (same as i.MX6).

Cc: yurovsky-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org
Cc: Shawn Guo <shawnguo-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Cc: Sascha Hauer <kernel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
Cc: Fabio Estevam <fabio.estevam-3arQi8VN3Tc@public.gmane.org>
Cc: Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Cc: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
Cc: Russell King <linux-I+IVW8TIWO2tmTQ+vhA3Yw@public.gmane.org>
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Signed-off-by: Andrey Smirnov <andrew.smirnov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 arch/arm/boot/dts/imx7s.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/imx7s.dtsi b/arch/arm/boot/dts/imx7s.dtsi
index 8ff2cbdd..c80d0db 100644
--- a/arch/arm/boot/dts/imx7s.dtsi
+++ b/arch/arm/boot/dts/imx7s.dtsi
@@ -509,7 +509,7 @@
 					anatop-min-bit-val = <8>;
 					anatop-min-voltage = <800000>;
 					anatop-max-voltage = <1200000>;
-					anatop-enable-bit = <31>;
+					anatop-enable-bit = <0>;
 				};
 			};
 
-- 
2.9.3

--
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

* Re: [PATCH 1/3] ARM: at91: flush the L2 cache before entering cpu idle
From: Alexandre Belloni @ 2017-01-10 16:30 UTC (permalink / raw)
  To: Wenyou Yang
  Cc: Mark Rutland, devicetree, Russell King, Wenyou Yang,
	Nicolas Ferre, linux-kernel, Rob Herring, linux-arm-kernel
In-Reply-To: <20170110161821.vp673jyfqx6s76pg@piout.net>

On 10/01/2017 at 17:18:21 +0100, Alexandre Belloni wrote :
> I though a bit more about it, and I don't really like the new compatible
> string. I don't feel this should be necessary.
> 
> What about the following:
> 
> diff --git a/arch/arm/mach-at91/pm.c b/arch/arm/mach-at91/pm.c
> index b4332b727e9c..0333aca63e44 100644
> --- a/arch/arm/mach-at91/pm.c
> +++ b/arch/arm/mach-at91/pm.c
> @@ -53,6 +53,7 @@ extern void at91_pinctrl_gpio_resume(void);
>  static struct {
>  	unsigned long uhp_udp_mask;
>  	int memctrl;
> +	bool has_l2_cache;
>  } at91_pm_data;
>  
>  void __iomem *at91_ramc_base[2];
> @@ -267,6 +268,11 @@ static void at91_ddr_standby(void)
>  	u32 lpr0, lpr1 = 0;
>  	u32 saved_lpr0, saved_lpr1 = 0;
>  
> +	if (at91_pm_data.has_l2_cache) {
> +		flush_cache_all();
> +		outer_disable();
> +	}
> +
>  	if (at91_ramc_base[1]) {
>  		saved_lpr1 = at91_ramc_read(1, AT91_DDRSDRC_LPR);
>  		lpr1 = saved_lpr1 & ~AT91_DDRSDRC_LPCB;
> @@ -287,6 +293,9 @@ static void at91_ddr_standby(void)
>  	at91_ramc_write(0, AT91_DDRSDRC_LPR, saved_lpr0);
>  	if (at91_ramc_base[1])
>  		at91_ramc_write(1, AT91_DDRSDRC_LPR, saved_lpr1);
> +
> +	if (at91_pm_data.has_l2_cache)
> +		outer_resume();
>  }
>  
>  /* We manage both DDRAM/SDRAM controllers, we need more than one value
>  * to
> @@ -353,6 +362,11 @@ static __init void at91_dt_ramc(void)
>  		return;
>  	}
>  
> +	np = of_find_compatible_node(NULL, NULL, "arm,pl310-cache");
> +	if (np)
> +		at91_pm_data.has_l2_cache = true;
> +	of_node_put(np);
> +
>  	at91_pm_set_standby(standby);
>  }
> 
> 
> This has the following benefits:
>  - everybody will have the fix, regardless of whether the dtb is updated
>  - has_l2_cache can be used later in at91_pm_suspend instead of calling
>    it unconditionnaly (I'll send a patch)
> 

I forgot to add that the added latency on at91sam9 and sama5d3 is exactly 5 instructions.

> 
> On 06/01/2017 at 14:59:45 +0800, Wenyou Yang wrote :
> > For the SoCs such as SAMA5D2 and SAMA5D4 which have L2 cache,
> > flush the L2 cache first before entering the cpu idle.
> > 
> > Signed-off-by: Wenyou Yang <wenyou.yang@atmel.com>
> > ---
> > 
> >  arch/arm/mach-at91/pm.c       | 19 +++++++++++++++++++
> >  drivers/memory/atmel-sdramc.c |  1 +
> >  2 files changed, 20 insertions(+)
> > 
> > diff --git a/arch/arm/mach-at91/pm.c b/arch/arm/mach-at91/pm.c
> > index b4332b727e9c..1a60dede1a01 100644
> > --- a/arch/arm/mach-at91/pm.c
> > +++ b/arch/arm/mach-at91/pm.c
> > @@ -289,6 +289,24 @@ static void at91_ddr_standby(void)
> >  		at91_ramc_write(1, AT91_DDRSDRC_LPR, saved_lpr1);
> >  }
> >  
> > +static void at91_ddr_cache_standby(void)
> > +{
> > +	u32 saved_lpr;
> > +
> > +	flush_cache_all();
> > +	outer_disable();
> > +
> > +	saved_lpr = at91_ramc_read(0, AT91_DDRSDRC_LPR);
> > +	at91_ramc_write(0, AT91_DDRSDRC_LPR, (saved_lpr &
> > +			(~AT91_DDRSDRC_LPCB)) | AT91_DDRSDRC_LPCB_SELF_REFRESH);
> > +
> > +	cpu_do_idle();
> > +
> > +	at91_ramc_write(0, AT91_DDRSDRC_LPR, saved_lpr);
> > +
> > +	outer_resume();
> > +}
> > +
> >  /* We manage both DDRAM/SDRAM controllers, we need more than one value to
> >   * remember.
> >   */
> > @@ -324,6 +342,7 @@ static const struct of_device_id const ramc_ids[] __initconst = {
> >  	{ .compatible = "atmel,at91sam9260-sdramc", .data = at91sam9_sdram_standby },
> >  	{ .compatible = "atmel,at91sam9g45-ddramc", .data = at91_ddr_standby },
> >  	{ .compatible = "atmel,sama5d3-ddramc", .data = at91_ddr_standby },
> > +	{ .compatible = "atmel,sama5d4-ddramc", .data = at91_ddr_cache_standby },
> >  	{ /*sentinel*/ }
> >  };
> >  
> > diff --git a/drivers/memory/atmel-sdramc.c b/drivers/memory/atmel-sdramc.c
> > index b418b39af180..7e5c5c6c1348 100644
> > --- a/drivers/memory/atmel-sdramc.c
> > +++ b/drivers/memory/atmel-sdramc.c
> > @@ -48,6 +48,7 @@ static const struct of_device_id atmel_ramc_of_match[] = {
> >  	{ .compatible = "atmel,at91sam9260-sdramc", .data = &at91rm9200_caps, },
> >  	{ .compatible = "atmel,at91sam9g45-ddramc", .data = &at91sam9g45_caps, },
> >  	{ .compatible = "atmel,sama5d3-ddramc", .data = &sama5d3_caps, },
> > +	{ .compatible = "atmel,sama5d4-ddramc", .data = &sama5d3_caps, },
> >  	{},
> >  };
> >  
> > -- 
> > 2.11.0
> > 
> 
> -- 
> Alexandre Belloni, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com

-- 
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

^ permalink raw reply

* Re: [PATCH 1/3] ARM: dts: da850: Add the cppi41 dma node
From: Sergei Shtylyov @ 2017-01-10 16:42 UTC (permalink / raw)
  To: David Lechner, Alexandre Bailon, nsekhar-l0cyMroinI0,
	khilman-rdvid1DuHRBWk0Htik3J/w
  Cc: vinod.koul-ral2JQCrhuEAvxtiuMwx3w,
	dmaengine-u79uwXL29TY76Z2rM5mHXA,
	linux-usb-u79uwXL29TY76Z2rM5mHXA, ptitiano-rdvid1DuHRBWk0Htik3J/w,
	tony-4v6yS6AI5VpBDgjK7y7TUQ, linux-omap-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	b-liu-l0cyMroinI0
In-Reply-To: <026deab8-8584-0744-3ec5-9346a7f6d16a-nq/r/kbU++upp/zk7JDF2g@public.gmane.org>

On 01/10/2017 07:15 PM, David Lechner wrote:

>>> This adds the device tree node for the cppi41 dma
>>> used by the usb otg controller present in the da850 family of SoC's.
>>>
>>> Signed-off-by: Alexandre Bailon <abailon-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
>>> ---
>>>  arch/arm/boot/dts/da850.dtsi | 16 ++++++++++++++++
>>>  1 file changed, 16 insertions(+)
>>>
>>> diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
>>> index 104155d..d6b406a 100644
>>> --- a/arch/arm/boot/dts/da850.dtsi
>>> +++ b/arch/arm/boot/dts/da850.dtsi
>>> @@ -403,6 +403,22 @@
>>>              phy-names = "usb-phy";
>>>              status = "disabled";
>>>          };
>>> +        cppi41dma: dma-controller@201000 {
>>> +            compatible = "ti,da8xx-cppi41";
>>> +            reg =  <0x200000 0x1000
>>
>>    I don't remember any DA8xx glue regs having to do with the CPPI 4.1...
>>
>>> +                0x201000 0x1000
>>> +                0x202000 0x1000
>>> +                0x204000 0x4000>;
>>> +            reg-names = "glue", "controller",
>>> +                    "scheduler", "queuemgr";
>>> +            interrupts = <58>;
>>> +            interrupt-names = "glue";
>>> +            #dma-cells = <2>;
>>> +            #dma-channels = <4>;
>>> +            #dma-requests = <256>;
>>> +            status = "disabled";
>>
>>    Why disabled? It doesn't use any external pins...
>
> Why enable it if musb node is not also enabled?

    Well, it's a good point.

>> [...]

MBR, Sergei

--
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/3] ARM: at91: flush the L2 cache before entering cpu idle
From: Jean-Jacques Hiblot @ 2017-01-10 16:50 UTC (permalink / raw)
  To: Alexandre Belloni
  Cc: Wenyou Yang, Mark Rutland, devicetree, Russell King, Wenyou Yang,
	Nicolas Ferre, Linux Kernel Mailing List, Rob Herring,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
In-Reply-To: <20170110161821.vp673jyfqx6s76pg-m++hUPXGwpdeoWH0uzbU5w@public.gmane.org>

2017-01-10 17:18 GMT+01:00 Alexandre Belloni
<alexandre.belloni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>:
> I though a bit more about it, and I don't really like the new compatible
> string. I don't feel this should be necessary.
>
> What about the following:
>
> diff --git a/arch/arm/mach-at91/pm.c b/arch/arm/mach-at91/pm.c
> index b4332b727e9c..0333aca63e44 100644
> --- a/arch/arm/mach-at91/pm.c
> +++ b/arch/arm/mach-at91/pm.c
> @@ -53,6 +53,7 @@ extern void at91_pinctrl_gpio_resume(void);
>  static struct {
>         unsigned long uhp_udp_mask;
>         int memctrl;
> +       bool has_l2_cache;
>  } at91_pm_data;
>
>  void __iomem *at91_ramc_base[2];
> @@ -267,6 +268,11 @@ static void at91_ddr_standby(void)
>         u32 lpr0, lpr1 = 0;
>         u32 saved_lpr0, saved_lpr1 = 0;
>

> +       if (at91_pm_data.has_l2_cache) {
> +               flush_cache_all();
what is the point of calling flush_cache_all() here ? Do we really
care that dirty data in L1 is written to DDR ? I may be missing
something but to me it's just extra latency.
> +               outer_disable();
It seems to me that if there's no L2 cache, then outer_disable()  is a
no-op. It could be called unconditionally.
> +       }
> +
>         if (at91_ramc_base[1]) {
>                 saved_lpr1 = at91_ramc_read(1, AT91_DDRSDRC_LPR);
>                 lpr1 = saved_lpr1 & ~AT91_DDRSDRC_LPCB;
> @@ -287,6 +293,9 @@ static void at91_ddr_standby(void)
>         at91_ramc_write(0, AT91_DDRSDRC_LPR, saved_lpr0);
>         if (at91_ramc_base[1])
>                 at91_ramc_write(1, AT91_DDRSDRC_LPR, saved_lpr1);
> +
> +       if (at91_pm_data.has_l2_cache)
> +               outer_resume();

same remark as for outer_disable()

Jean-Jacques

>  }
>
>  /* We manage both DDRAM/SDRAM controllers, we need more than one value
>  * to
> @@ -353,6 +362,11 @@ static __init void at91_dt_ramc(void)
>                 return;
>         }
>
> +       np = of_find_compatible_node(NULL, NULL, "arm,pl310-cache");
> +       if (np)
> +               at91_pm_data.has_l2_cache = true;
> +       of_node_put(np);
> +
>         at91_pm_set_standby(standby);
>  }
>
>
> This has the following benefits:
>  - everybody will have the fix, regardless of whether the dtb is updated
>  - has_l2_cache can be used later in at91_pm_suspend instead of calling
>    it unconditionnaly (I'll send a patch)
>
>
> On 06/01/2017 at 14:59:45 +0800, Wenyou Yang wrote :
>> For the SoCs such as SAMA5D2 and SAMA5D4 which have L2 cache,
>> flush the L2 cache first before entering the cpu idle.
>>
>> Signed-off-by: Wenyou Yang <wenyou.yang-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>
>> ---
>>
>>  arch/arm/mach-at91/pm.c       | 19 +++++++++++++++++++
>>  drivers/memory/atmel-sdramc.c |  1 +
>>  2 files changed, 20 insertions(+)
>>
>> diff --git a/arch/arm/mach-at91/pm.c b/arch/arm/mach-at91/pm.c
>> index b4332b727e9c..1a60dede1a01 100644
>> --- a/arch/arm/mach-at91/pm.c
>> +++ b/arch/arm/mach-at91/pm.c
>> @@ -289,6 +289,24 @@ static void at91_ddr_standby(void)
>>               at91_ramc_write(1, AT91_DDRSDRC_LPR, saved_lpr1);
>>  }
>>
>> +static void at91_ddr_cache_standby(void)
>> +{
>> +     u32 saved_lpr;
>> +
>> +     flush_cache_all();
>> +     outer_disable();
>> +
>> +     saved_lpr = at91_ramc_read(0, AT91_DDRSDRC_LPR);
>> +     at91_ramc_write(0, AT91_DDRSDRC_LPR, (saved_lpr &
>> +                     (~AT91_DDRSDRC_LPCB)) | AT91_DDRSDRC_LPCB_SELF_REFRESH);
>> +
>> +     cpu_do_idle();
>> +
>> +     at91_ramc_write(0, AT91_DDRSDRC_LPR, saved_lpr);
>> +
>> +     outer_resume();
>> +}
>> +
>>  /* We manage both DDRAM/SDRAM controllers, we need more than one value to
>>   * remember.
>>   */
>> @@ -324,6 +342,7 @@ static const struct of_device_id const ramc_ids[] __initconst = {
>>       { .compatible = "atmel,at91sam9260-sdramc", .data = at91sam9_sdram_standby },
>>       { .compatible = "atmel,at91sam9g45-ddramc", .data = at91_ddr_standby },
>>       { .compatible = "atmel,sama5d3-ddramc", .data = at91_ddr_standby },
>> +     { .compatible = "atmel,sama5d4-ddramc", .data = at91_ddr_cache_standby },
>>       { /*sentinel*/ }
>>  };
>>
>> diff --git a/drivers/memory/atmel-sdramc.c b/drivers/memory/atmel-sdramc.c
>> index b418b39af180..7e5c5c6c1348 100644
>> --- a/drivers/memory/atmel-sdramc.c
>> +++ b/drivers/memory/atmel-sdramc.c
>> @@ -48,6 +48,7 @@ static const struct of_device_id atmel_ramc_of_match[] = {
>>       { .compatible = "atmel,at91sam9260-sdramc", .data = &at91rm9200_caps, },
>>       { .compatible = "atmel,at91sam9g45-ddramc", .data = &at91sam9g45_caps, },
>>       { .compatible = "atmel,sama5d3-ddramc", .data = &sama5d3_caps, },
>> +     { .compatible = "atmel,sama5d4-ddramc", .data = &sama5d3_caps, },
>>       {},
>>  };
>>
>> --
>> 2.11.0
>>
>
> --
> Alexandre Belloni, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
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 v3 1/3] arm64: dts: exynos5433: add DECON_TV node
From: Krzysztof Kozlowski @ 2017-01-10 17:11 UTC (permalink / raw)
  To: Andrzej Hajda
  Cc: linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA, Krzysztof Kozlowski,
	Bartlomiej Zolnierkiewicz, Marek Szyprowski, Inki Dae,
	Rob Herring, Mark Rutland, Javier Martinez Canillas,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Andi Shyti, Chanwoo Choi
In-Reply-To: <1484036664-15951-1-git-send-email-a.hajda-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>

On Tue, Jan 10, 2017 at 09:24:22AM +0100, Andrzej Hajda wrote:
> DECON_TV is 2nd display controller on Exynos5433, used in HDMI path
> or 2nd DSI path.
> 
> Signed-off-by: Andrzej Hajda <a.hajda-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> Reviewed-by: Javier Martinez Canillas <javier-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>
> Reviewed-by: Chanwoo Choi <cw00.choi-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> Tested-by: Hoegeun Kwon <hoegeun.kwon-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> ---
> 
> Hi Krzysztof,
> 
> These patches are based on latest patches separating tm2 and tm2e and
> touchscreen patches. I hope this is good base.
> Thanks all for quick response/review.
>

Thanks for fixing the issues. Also thank you for trying to fix the
subject. However this is not a full fix. Please run:
$ git log --oneline arch/arm64/boot/dts/exynos/

Why I am asking about this? I am trying to have it consistent so
going through history would be easier. Of course we might discuss what
prefix should be used. Some other platforms use "arm64: dts:
architecture:", some use "arm64: dts: specific_chip_family:". For exynos
we are trying to keep it just "exynos". Anyway regardless of the chosen
prefix itself, after choosing some pattern, the consistency helps.

When people keep sending patches with wrong title, then after every
git-am I have to amend the commit and fix it. It is not difficult but
annoying.

Actually before I did not complain about it... but recently (this cycle)
a lot of Samsung folks are sending wrong subjects. Weird.

So just use the command and look at history.

Best regards,
Krzysztof
--
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 v3 0/4] hwmon: adc128d818: Support missing operation modes
From: Guenter Roeck @ 2017-01-10 17:17 UTC (permalink / raw)
  To: Alexander Koch
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-hwmon-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Mark Rutland,
	Jean Delvare, Michael Hornung, devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20170106103817.11588-1-mail-y2PnNNZjvYd4VEKF+Mn3m16hYfS7NtTn@public.gmane.org>

On Fri, Jan 06, 2017 at 11:38:13AM +0100, Alexander Koch wrote:
> The ADC128D818 offers four different chip operation modes which vary in the
> number and measurement types of the available input signals (see datasheet
> sec. 8.4.1).
> 
> The current version of the driver only supports the default chip operation
> mode (mode 0), providing seven analog values and a temperature reading.
> 
> This patch series adds support for operation modes 1-3, selectable through
> the device tree attribute 'ti,mode':
> 
>         adc1: adc128d818@1d {
>                 compatible = "ti,adc128d818";
>                 reg = <0x1d>;
>                 mode = <1>;
>         };
> 
> The changes are transparent as the driver defaults to keeping the currently
> active operation mode if no mode is specified via device tree (which is
> mode 0 on chip initialization).
> 
> 
> Changes from v2:
>  - Omit device attribute refactoring (for checkpatch.pl), as requested by
>    maintainer
>  - Add vendor prefix 'ti,' for mode property in device tree
>  - Drop size indication for mode property in device tree
>  - Preserve chip operation mode if none specified in devicetree
>  - Fix missing '\n' in dev_err() calls
> 
> Changes from v1:
>  - Add bindings document as first patch
>  - Preserve logical atomicity of code changes
>  - Improve sysfs device node handling (use is_visible() instead of
>    duplicate attribute list)
>  - Add trivial code refactoring stage for checkpatch.pl to succeed
> 
> 
> Alexander Koch (4):
>   devicetree: hwmon: Add bindings for ADC128D818
>   hwmon: adc128d818: Implement mode selection via dt
>   hwmon: adc128d818: Support operation modes 1-3
>   hwmon: adc128d818: Preserve operation mode
> 
>  .../devicetree/bindings/hwmon/adc128d818.txt       |  39 ++++++
>  drivers/hwmon/adc128d818.c                         | 147 +++++++++++++++------
>  2 files changed, 149 insertions(+), 37 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/hwmon/adc128d818.txt
> 
Series applied to -next.

Thanks,
Guenter
--
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/3] ARM: at91: flush the L2 cache before entering cpu idle
From: Alexandre Belloni @ 2017-01-10 17:22 UTC (permalink / raw)
  To: Jean-Jacques Hiblot
  Cc: Wenyou Yang, Mark Rutland, devicetree, Russell King, Wenyou Yang,
	Nicolas Ferre, Linux Kernel Mailing List, Rob Herring,
	linux-arm-kernel@lists.infradead.org
In-Reply-To: <CACh+v5NX9Da__zXtnVGgbO9wYAzEJy-8rjVPHdQx_F6QDk+xZg@mail.gmail.com>

On 10/01/2017 at 17:50:58 +0100, Jean-Jacques Hiblot wrote :
> 2017-01-10 17:18 GMT+01:00 Alexandre Belloni
> <alexandre.belloni@free-electrons.com>:
> > I though a bit more about it, and I don't really like the new compatible
> > string. I don't feel this should be necessary.
> >
> > What about the following:
> >
> > diff --git a/arch/arm/mach-at91/pm.c b/arch/arm/mach-at91/pm.c
> > index b4332b727e9c..0333aca63e44 100644
> > --- a/arch/arm/mach-at91/pm.c
> > +++ b/arch/arm/mach-at91/pm.c
> > @@ -53,6 +53,7 @@ extern void at91_pinctrl_gpio_resume(void);
> >  static struct {
> >         unsigned long uhp_udp_mask;
> >         int memctrl;
> > +       bool has_l2_cache;
> >  } at91_pm_data;
> >
> >  void __iomem *at91_ramc_base[2];
> > @@ -267,6 +268,11 @@ static void at91_ddr_standby(void)
> >         u32 lpr0, lpr1 = 0;
> >         u32 saved_lpr0, saved_lpr1 = 0;
> >
> 
> > +       if (at91_pm_data.has_l2_cache) {
> > +               flush_cache_all();
> what is the point of calling flush_cache_all() here ? Do we really
> care that dirty data in L1 is written to DDR ? I may be missing
> something but to me it's just extra latency.

I agree that this one is the main problem.

> > +               outer_disable();
> It seems to me that if there's no L2 cache, then outer_disable()  is a
> no-op. It could be called unconditionally.

It is not on sama5, it will jump to outer_disable which will at least
save the context and restore it

> > +       }
> > +
> >         if (at91_ramc_base[1]) {
> >                 saved_lpr1 = at91_ramc_read(1, AT91_DDRSDRC_LPR);
> >                 lpr1 = saved_lpr1 & ~AT91_DDRSDRC_LPCB;
> > @@ -287,6 +293,9 @@ static void at91_ddr_standby(void)
> >         at91_ramc_write(0, AT91_DDRSDRC_LPR, saved_lpr0);
> >         if (at91_ramc_base[1])
> >                 at91_ramc_write(1, AT91_DDRSDRC_LPR, saved_lpr1);
> > +
> > +       if (at91_pm_data.has_l2_cache)
> > +               outer_resume();
> 
> same remark as for outer_disable()

It is not either but this is a macro and I admit testing has_l2_cache is
superfluous.


-- 
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

^ permalink raw reply

* Re: [PATCH v2 2/2] media: rc: add driver for IR remote receiver on MT7623 SoC
From: Sean Young @ 2017-01-10 17:23 UTC (permalink / raw)
  To: Sean Wang
  Cc: mchehab-JPH+aEBZ4P+UEJcrhfAQsw, hdegoede-H+wXaHxf7aLQT0dZR+AlfA,
	hkallweit1-Re5JQEeQqe8AvxtiuMwx3w, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	mark.rutland-5wv7dgnIgG8, matthias.bgg-Re5JQEeQqe8AvxtiuMwx3w,
	andi.shyti-Sze3O3UU22JBDgjK7y7TUQ,
	hverkuil-qWit8jRvyhVmR6Xm/wNWPw,
	ivo.g.dimitrov.75-Re5JQEeQqe8AvxtiuMwx3w,
	linux-media-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	keyhaede-Re5JQEeQqe8AvxtiuMwx3w
In-Reply-To: <1484056789.4057.17.camel@mtkswgap22>

Hi Sean,

The driver is looking very good, we are looking at minor details now.

On Tue, Jan 10, 2017 at 09:59:49PM +0800, Sean Wang wrote:
> On Tue, 2017-01-10 at 11:09 +0000, Sean Young wrote:
> 
> > > +#include <linux/clk.h>
> > > +#include <linux/interrupt.h>
> > > +#include <linux/module.h>
> > > +#include <linux/of_platform.h>
> > > +#include <linux/reset.h>
> > > +#include <media/rc-core.h>
> > > +
> > > +#define MTK_IR_DEV KBUILD_MODNAME
> > 
> > You could remove this #define and just use KBUILD_MODNAME
> 
> I preferred to use MTK_IR_DEV internally that helps
> renaming in the future if necessary.

ok.

> >  
> > > +
> > > +/* Register to enable PWM and IR */
> > > +#define MTK_CONFIG_HIGH_REG       0x0c
> > > +/* Enable IR pulse width detection */
> > > +#define MTK_PWM_EN		  BIT(13)
> > > +/* Enable IR hardware function */
> > > +#define MTK_IR_EN		  BIT(0)
> > > +
> > > +/* Register to setting sample period */
> > > +#define MTK_CONFIG_LOW_REG        0x10
> > > +/* Field to set sample period */
> > > +#define CHK_PERIOD		  DIV_ROUND_CLOSEST(MTK_IR_SAMPLE,  \
> > > +						    MTK_IR_CLK_PERIOD)
> > > +#define MTK_CHK_PERIOD            (((CHK_PERIOD) << 8) & (GENMASK(20, 8)))
> > > +#define MTK_CHK_PERIOD_MASK	  (GENMASK(20, 8))
> > > +
> > > +/* Register to clear state of state machine */
> > > +#define MTK_IRCLR_REG             0x20
> > > +/* Bit to restart IR receiving */
> > > +#define MTK_IRCLR		  BIT(0)
> > > +
> > > +/* Register containing pulse width data */
> > > +#define MTK_CHKDATA_REG(i)        (0x88 + 4 * (i))
> > > +#define MTK_WIDTH_MASK		  (GENMASK(7, 0))
> > > +
> > > +/* Register to enable IR interrupt */
> > > +#define MTK_IRINT_EN_REG          0xcc
> > > +/* Bit to enable interrupt */
> > > +#define MTK_IRINT_EN		  BIT(0)
> > > +
> > > +/* Register to ack IR interrupt */
> > > +#define MTK_IRINT_CLR_REG         0xd0
> > > +/* Bit to clear interrupt status */
> > > +#define MTK_IRINT_CLR		  BIT(0)
> > > +
> > > +/* Maximum count of samples */
> > > +#define MTK_MAX_SAMPLES		  0xff
> > > +/* Indicate the end of IR message */
> > > +#define MTK_IR_END(v, p)	  ((v) == MTK_MAX_SAMPLES && (p) == 0)
> > > +/* Number of registers to record the pulse width */
> > > +#define MTK_CHKDATA_SZ		  17
> > > +/* Source clock frequency */
> > > +#define MTK_IR_BASE_CLK		  273000000
> > > +/* Frequency after IR internal divider */
> > > +#define MTK_IR_CLK_FREQ		  (MTK_IR_BASE_CLK / 4)
> 
> > > +static irqreturn_t mtk_ir_irq(int irqno, void *dev_id)
> > > +{
> > > +	struct mtk_ir *ir = dev_id;
> > > +	u8  wid = 0;
> > > +	u32 i, j, val;
> > > +	DEFINE_IR_RAW_EVENT(rawir);
> > > +
> > > +	mtk_irq_disable(ir, MTK_IRINT_EN);
> > 
> > The kernel guarantees that calls to the interrupt handler are serialised,
> > no need to disable the interrupt in the handler.
> 
> agreed. I will save the mtk irq disable/enable and retest again.
> 
> 
> > > +
> > > +	/* Reset decoder state machine */
> > > +	ir_raw_event_reset(ir->rc);
> > 
> > Not needed.
> 
> 
> two reasons I added the line here
> 
> 1) 
> I thought it is possible the decoder goes to the
> middle state when getting the data not belonged
> to the protocol. If so, that would cause the decoding
> fails in the next time receiving the valid protocol data.

The last IR event submitted will always be a long space, that's enough
to reset the decoders. Adding a ir_raw_event_reset() will do this
more explicitly, rather than their state machines resetting themselves
through the trailing space.

> 2) 
> the mtk hardware register always contains the start of 
> IR message. So force to sync the state between 
> HW and ir-core.
> 
> 
> 
> > > +
> > > +	/* First message must be pulse */
> > > +	rawir.pulse = false;
> > 
> > pulse = true?
> 
> becasue of rawir.pulse = !rawir.pulse does as below
> so the initial value is set as false.

Ah, sorry, of course. :)

> > > +
> > > +	/* Handle all pulse and space IR controller captures */
> > > +	for (i = 0 ; i < MTK_CHKDATA_SZ ; i++) {
> > > +		val = mtk_r32(ir, MTK_CHKDATA_REG(i));
> > > +		dev_dbg(ir->dev, "@reg%d=0x%08x\n", i, val);
> > > +
> > > +		for (j = 0 ; j < 4 ; j++) {
> > > +			wid = (val & (MTK_WIDTH_MASK << j * 8)) >> j * 8;
> > > +			rawir.pulse = !rawir.pulse;
> > > +			rawir.duration = wid * (MTK_IR_SAMPLE + 1);
> > > +			ir_raw_event_store_with_filter(ir->rc, &rawir);
> > > +		}
> > 
> > In v1 you would break out of the loop if the ir message was shorter, but
> > now you are always passing on 68 pulses and spaces. Is that right?
> 
> as I asked in the previous mail list as below i copied from it, so i
> made some changes ...
> 
> """""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""
> > I had another question. I found multiple and same IR messages being
> > received when using SONY remote controller. Should driver needs to
> > report each message or only one of these to the upper layer ?
> 
> In general the driver shouldn't try to change any IR message, this
> should be done in rc-core if necessary.
> 
> rc-core should handle this correctly. If the same key is received twice
> within IR_KEYPRESS_TIMEOUT (250ms) then it not reported to the input
> layer.
> """"""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""
> 
> for example:
> the 68 pulse/spaces might contains 2.x IR messages when I
> pressed one key on SONY remote control. 
> 
> the v1 proposed is passing only one IR message into ir-core ; 
> the v2 done is passing all IR messages even including the last
> incomplete message into ir-core. 

Yes, agreed. Sorry if I wasn't clear. I just wanted to make sure you've
thought about what happens when the IR message is short (e.g. rc-5 with
23 pulse-spaces). Are the remaining registers 0 or do we get stale data
from a previous transmit?

> But I was still afraid the state machine can't  go back to initial state
> after receiving these incomplete data. 
> 
> So the ir_raw_event_reset() call in the beginning of ISR seems becoming
> more important.
> 
> > > +	}
> > > +
> > > +	/* The maximum number of edges the IR controller can
> > > +	 * hold is MTK_CHKDATA_SZ * 4. So if received IR messages
> > > +	 * is over the limit, the last incomplete IR message would
> > > +	 * be appended trailing space and still would be sent into
> > > +	 * ir-rc-raw to decode. That helps it is possible that it
> > > +	 * has enough information to decode a scancode even if the
> > > +	 * trailing end of the message is missing.
> > > +	 */
> > > +	if (!MTK_IR_END(wid, rawir.pulse)) {
> > > +		rawir.pulse = false;
> > > +		rawir.duration = MTK_MAX_SAMPLES * (MTK_IR_SAMPLE + 1);
> > > +		ir_raw_event_store_with_filter(ir->rc, &rawir);
> > > +	}

See here you add a long space if one was not added already.

> > > +
> > > +	ir_raw_event_handle(ir->rc);
> > > +
> > > +	/* Restart controller for the next receive */
> > > +	mtk_w32_mask(ir, 0x1, MTK_IRCLR, MTK_IRCLR_REG);
> > > +
> > > +	/* Clear interrupt status */
> > > +	mtk_w32_mask(ir, 0x1, MTK_IRINT_CLR, MTK_IRINT_CLR_REG);
> > > +
> > > +	/* Enable interrupt */
> > > +	mtk_irq_enable(ir, MTK_IRINT_EN);
> > > +
> > > +	return IRQ_HANDLED;
> > > +}
> > > +
> > > +static int mtk_ir_probe(struct platform_device *pdev)
> > > +{
> > > +	struct device *dev = &pdev->dev;
> > > +	struct device_node *dn = dev->of_node;
> > > +	struct resource *res;
> > > +	struct mtk_ir *ir;
> > > +	u32 val;
> > > +	int ret = 0;
> > > +	const char *map_name;
> > > +
> > > +	ir = devm_kzalloc(dev, sizeof(struct mtk_ir), GFP_KERNEL);
> > > +	if (!ir)
> > > +		return -ENOMEM;
> > > +
> > > +	ir->dev = dev;
> > > +
> > > +	if (!of_device_is_compatible(dn, "mediatek,mt7623-cir"))
> > > +		return -ENODEV;
> > > +
> > > +	ir->clk = devm_clk_get(dev, "clk");
> > > +	if (IS_ERR(ir->clk)) {
> > > +		dev_err(dev, "failed to get a ir clock.\n");
> > > +		return PTR_ERR(ir->clk);
> > > +	}
> > > +
> > > +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > > +	ir->base = devm_ioremap_resource(dev, res);
> > > +	if (IS_ERR(ir->base)) {
> > > +		dev_err(dev, "failed to map registers\n");
> > > +		return PTR_ERR(ir->base);
> > > +	}
> > > +
> > > +	ir->rc = devm_rc_allocate_device(dev, RC_DRIVER_IR_RAW);
> > > +	if (!ir->rc) {
> > > +		dev_err(dev, "failed to allocate device\n");
> > > +		return -ENOMEM;
> > > +	}
> > > +
> > > +	ir->rc->priv = ir;
> > > +	ir->rc->input_name = MTK_IR_DEV;
> > > +	ir->rc->input_phys = MTK_IR_DEV "/input0";
> > > +	ir->rc->input_id.bustype = BUS_HOST;
> > > +	ir->rc->input_id.vendor = 0x0001;
> > > +	ir->rc->input_id.product = 0x0001;
> > > +	ir->rc->input_id.version = 0x0001;
> > > +	map_name = of_get_property(dn, "linux,rc-map-name", NULL);
> > > +	ir->rc->map_name = map_name ?: RC_MAP_EMPTY;
> > > +	ir->rc->dev.parent = dev;
> > > +	ir->rc->driver_name = MTK_IR_DEV;
> > > +	ir->rc->allowed_protocols = RC_BIT_ALL;
> > > +	ir->rc->rx_resolution = MTK_IR_SAMPLE;
> > > +	ir->rc->timeout = MTK_MAX_SAMPLES * (MTK_IR_SAMPLE + 1);
> > > +
> > > +	ret = devm_rc_register_device(dev, ir->rc);
> > 
> > Here you do devm_rc_register_device()
> 
> does it have problem ?

Sorry, no. I just wanted to highlight wrt a comment below.
> 
> 
> > > +	if (ret) {
> > > +		dev_err(dev, "failed to register rc device\n");
> > > +		return ret;
> > > +	}
> > > +
> > > +	platform_set_drvdata(pdev, ir);
> > > +
> > > +	ir->irq = platform_get_irq(pdev, 0);
> > > +	if (ir->irq < 0) {
> > > +		dev_err(dev, "no irq resource\n");
> > > +		return -ENODEV;
> > > +	}
> > > +
> > > +	/* Enable interrupt after proper hardware
> > > +	 * setup and IRQ handler registration
> > > +	 */
> > > +	if (clk_prepare_enable(ir->clk)) {
> > > +		dev_err(dev, "try to enable ir_clk failed\n");
> > > +		ret = -EINVAL;
> > > +		goto exit_clkdisable_clk;
> > > +	}
> > > +
> > > +	mtk_irq_disable(ir, MTK_IRINT_EN);
> > > +
> > > +	ret = devm_request_irq(dev, ir->irq, mtk_ir_irq, 0, MTK_IR_DEV, ir);
> > > +	if (ret) {
> > > +		dev_err(dev, "failed request irq\n");
> > > +		goto exit_clkdisable_clk;
> > > +	}
> > > +
> > > +	/* Enable IR and PWM */
> > > +	val = mtk_r32(ir, MTK_CONFIG_HIGH_REG);
> > > +	val |= MTK_PWM_EN | MTK_IR_EN;
> > > +	mtk_w32(ir, val, MTK_CONFIG_HIGH_REG);
> > > +
> > > +	/* Setting sample period */
> > > +	mtk_w32_mask(ir, MTK_CHK_PERIOD, MTK_CHK_PERIOD_MASK,
> > > +		     MTK_CONFIG_LOW_REG);
> > > +
> > > +	mtk_irq_enable(ir, MTK_IRINT_EN);
> > > +
> > > +	dev_info(dev, "Initialized MT7623 IR driver, sample period = %luus\n",
> > > +		 DIV_ROUND_CLOSEST(MTK_IR_SAMPLE, 1000));
> > > +
> > > +	return 0;
> > > +
> > > +exit_clkdisable_clk:
> > > +	clk_disable_unprepare(ir->clk);
> > > +
> > > +	return ret;
> > > +}
> > > +
> > > +static int mtk_ir_remove(struct platform_device *pdev)
> > > +{
> > > +	struct mtk_ir *ir = platform_get_drvdata(pdev);
> > > +
> > > +	/* Avoid contention between remove handler and
> > > +	 * IRQ handler so that disabling IR interrupt and
> > > +	 * waiting for pending IRQ handler to complete
> > > +	 */
> > > +	mtk_irq_disable(ir, MTK_IRINT_EN);
> > > +	synchronize_irq(ir->irq);
> > > +
> > > +	clk_disable_unprepare(ir->clk);
> > > +
> > > +	rc_unregister_device(ir->rc);
> > 
> > Yet here you explicitly call rc_unregister_device(). Since it was registered
> > with the devm call, this call is not needed and will lead to double frees etc
> 
> bug :( .  I will fix it ..
> 
> > > +
> > > +	return 0;
> > > +}
> > > +
> > > +static const struct of_device_id mtk_ir_match[] = {
> > > +	{ .compatible = "mediatek,mt7623-cir" },
> > > +	{},
> > > +};
> > > +MODULE_DEVICE_TABLE(of, mtk_ir_match);
> > > +
> > > +static struct platform_driver mtk_ir_driver = {
> > > +	.probe          = mtk_ir_probe,
> > > +	.remove         = mtk_ir_remove,
> > > +	.driver = {
> > > +		.name = MTK_IR_DEV,
> > > +		.of_match_table = mtk_ir_match,
> > > +	},
> > > +};
> > > +
> > > +module_platform_driver(mtk_ir_driver);
> > > +
> > > +MODULE_DESCRIPTION("Mediatek IR Receiver Controller Driver");
> > > +MODULE_AUTHOR("Sean Wang <sean.wang-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>");
> > > +MODULE_LICENSE("GPL");
> > > -- 
> > > 2.7.4
> > > 
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
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/2] ARM: dts: imx6: Specify 'anatop-enable-bit' where appropriate
From: Fabio Estevam @ 2017-01-10 17:28 UTC (permalink / raw)
  To: Andrey Smirnov
  Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	Mark Rutland, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Russell King, linux-kernel, Rob Herring, Sascha Hauer,
	Fabio Estevam, Shawn Guo, yurovsky-Re5JQEeQqe8AvxtiuMwx3w
In-Reply-To: <20170110163015.22444-1-andrew.smirnov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

On Tue, Jan 10, 2017 at 2:30 PM, Andrey Smirnov
<andrew.smirnov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> ENABLE_LINREG bit is implemented by 3P0, 1P1 and 2P5 regulators on
> i.MX6. This property is present in similar code in Fresscale BSP and
> made its way upstream in imx6ul.dtsi, so this patch adds this property
> to the rest of i.MX6 family for completness.

Please see:
https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/patch/arch/arm/boot/dts/imx6ul.dtsi?id=27958ccdf29e9971732e02494b48be54b0691269
--
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


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