* Re: [PATCH v10 00/13] Introduce framework for SLIMbus device driver
From: Greg Kroah-Hartman @ 2017-12-13 9:25 UTC (permalink / raw)
To: srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A
Cc: Mark Brown, alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
sdharia-sgV2jX0FEOL9JmXXK+q4OQ, Rob Herring, Mark Rutland,
Jonathan Corbet, pombredanne-od1rfyK75/E,
j.neuschaefer-hi6Y0CQ0nG0, linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-doc-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20171211234307.14465-1-srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
On Mon, Dec 11, 2017 at 11:42:54PM +0000, srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org wrote:
> From: Srinivas Kandagatla <srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>
> SLIMbus (Serial Low Power Interchip Media Bus) is a specification
> developed by MIPI (Mobile Industry Processor Interface) alliance.
> SLIMbus is a 2-wire implementation, which is used to communicate with
> peripheral components like audio-codec.
> SLIMbus uses Time-Division-Multiplexing to accommodate multiple data
> channels, and control channel. Control channel has messages to do
> device-enumeration, messages to send/receive control-data to/from
> SLIMbus devices, messages for port/channel management, and messages to
> do bandwidth allocation.
> Framework is introduced to support multiple instances of the bus
> (1 controller per bus), and multiple slave devices per controller.
> SPI and I2C frameworks, and comments from last time when I submitted
> the patches were referred-to while working on this framework.
>
> These patchsets introduce device-management, OF helpers, and messaging
> APIs, controller driver for Qualcomm's SLIMbus controller, and
> clock-pause feature for entering/exiting low-power mode for SLIMbus.
> Framework patches to do channel, port and bandwidth
> management are work-in-progress and will be sent out once these
> initial patches are accepted.
>
> These patchsets were tested on IFC6410 board with Qualcomm APQ8064
> processor using the controller driver, and a WCD9310 codec.
>
> v9: https://lkml.org/lkml/2017/12/7/289
>
> Changes from v9 to v10:
> * Added kernel-doc reference into slimbus driver api doc suggested by
> Jonathan Corbet
These all look good to me. I can take this through my tree if I get the
ack from Mark for the regmap changes.
thanks,
greg k-h
--
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 4/4] regulator: core: Balance coupled regulators voltages
From: Maciej Purski @ 2017-12-13 9:25 UTC (permalink / raw)
To: Mark Brown
Cc: linux-kernel, devicetree, Liam Girdwood, Rob Herring,
Mark Rutland, Marek Szyprowski, Bartlomiej Zolnierkiewicz
In-Reply-To: <20171212115427.GG16323@sirena.org.uk>
On 12/12/2017 12:54 PM, Mark Brown wrote:
> On Thu, Dec 07, 2017 at 10:46:15AM +0100, Maciej Purski wrote:
>
>> @@ -2447,10 +2482,9 @@ static int _regulator_is_enabled(struct regulator_dev *rdev)
>> return rdev->desc->ops->is_enabled(rdev);
>> }
>>
>> -static int _regulator_list_voltage(struct regulator *regulator,
>> - unsigned selector, int lock)
>> +static int _regulator_list_voltage(struct regulator_dev *rdev,
>> + unsigned selector, int lock)
>> {
>
> Please split this refactoring of _list_voltage() into a separate patch
> for ease of review. It can go in separately which will make this change
> smaller and easier to review.
>
>> @@ -2928,6 +2961,35 @@ static int regulator_set_voltage_unlocked(struct regulator *regulator,
>> if (ret < 0)
>> goto out2;
>>
>> + /*
>> + * If the regulator is not coupled just set voltage normally, else
>> + * return after changing consumer demands without changing voltage.
>> + * This will be handled outside the function
>> + * by regulator_balance_coupled()
>> + */
>> + if (!rdev->coupling_desc) {
>> + ret = regulator_set_voltage_rdev(regulator->rdev,
>> + min_uV, max_uV);
>> + if (ret < 0)
>> + goto out2;
>> + }
>
> As I think I said on the previous version I'm not enthusiastic about
> having two separate code paths for setting the voltage, it makes it much
> more likely that things will break especially given how rare coupled
> regulators are. It would be cleaner to make uncoupled regulators just
> be a special case of coupled regulators, that way more of the code is
> shared. To that end I'd adjust the code so that we always have a
> coupling descriptor and then handle the case where there's only one
> regulator described in there.
>
Do you have any suggestion, how should I implement that path? The thing which
makes it more complicated is locking, because set_voltage_unlocked is done under
one regulator's mutex and its suppliers, while balance procedure locks every
coupled regulator without its suppliers. The suppliers for a single regulator
are locked when setting a single regulator's voltage takes place.
^ permalink raw reply
* Re: [PATCH v2 3/4] thermal: armada: add support for CP110
From: Gregory CLEMENT @ 2017-12-13 9:21 UTC (permalink / raw)
To: Baruch Siach
Cc: Miquel RAYNAL, devicetree-u79uwXL29TY76Z2rM5mHXA, Jason Cooper,
Andrew Lunn, linux-pm-u79uwXL29TY76Z2rM5mHXA, Russell King,
Eduardo Valentin, Ezequiel Garcia, Zhang Rui,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
Sebastian Hesselbarth
In-Reply-To: <20171213091040.jwsphlax4yidm4qp-MwjkAAnuF3khR1HGirfZ1z4kX+cae0hd@public.gmane.org>
Hi Baruch,
On mer., déc. 13 2017, Baruch Siach <baruch-NswTu9S1W3P6gbPvEgmw2w@public.gmane.org> wrote:
> Hi Miquel,
>
> On Wed, Dec 13, 2017 at 09:55:01AM +0100, Miquel RAYNAL wrote:
>> > > > How would a separate init_sensor routine improve things?
>> > >
>> > > So yes please do it, thanks to this you won't have to add the
>> > > control_msb_offset member and can use a clean function. Moreover if
>> > > in the future we see some usefulness for this LSB register then we
>> > > could use the new compatible for the Armada 38x.
>> >
>> > There are two separate issues here:
>> >
>> > 1. DT binding
>> >
>> > 2. init_sensor callback implementation
>> >
>> > We both agree on #1. The A38x and CP110 need separate compatible
>> > strings. In case we want to access the LSB control register on Armada
>> > 38x, we will need yet another compatible string
>> > (marvell,armada380-v2-thermal maybe?).
>> >
>> > As for #2, I'm all for sharing as much code as possible. I find the
>> > vendor kernel approach of duplicating the init routines[1] unhelpful
>> > as it violates the DRY principle. The differences between
>> > armada380_init_sensor() and cp110_init_sensor() are minor. In my
>> > opinion, these differences should be expressed explicitly in the
>> > armada_thermal_data, in a similar way to my suggested
>> > control_msb_offset field. The vendor code hides these differences in
>> > slight variations of duplicated code.
>> >
>> > What is the advantage of a separate init routine?
>>
>> The advantage is that is the very near future I plan to add the
>> overheat interrupt only on CP110 (not on 38x) and this needs some
>> initialization. So if we don't make different routines now, I will
>> have to do it right after.
>
> I don't think so. The code of these functions in the vendor kernel overheat
> support implementation is the same, duplicated. The variations are only in
> registers/bits offsets. A single routine with one or two added
> armada_thermal_data fields would be much easier to comprehend and maintain.
>
>> What would be fine is to have the shared code in a separate function,
>> like it is done in Marvell kernel. What do you think about that?
>
> The Marvell code does not "share" the code. Separate functions means
> duplicated code that obscures the hardware details, making maintenance harder
> on the long run.
Well, Miquel speak about writting new code, so I don't see why you refer
the Marvell LSP code. Also, I don't see how having common function will
duplicate the code.
Gregory
>
> https://en.wikipedia.org/wiki/Don%27t_repeat_yourself
>
> baruch
>
> --
> http://baruch.siach.name/blog/ ~. .~ Tk Open Systems
> =}------------------------------------------------ooO--U--Ooo------------{=
> - baruch-NswTu9S1W3P6gbPvEgmw2w@public.gmane.org - tel: +972.2.679.5364, http://www.tkos.co.il -
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
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 v2 3/4] thermal: armada: add support for CP110
From: Gregory CLEMENT @ 2017-12-13 9:13 UTC (permalink / raw)
To: Baruch Siach
Cc: Ezequiel Garcia, Miquel RAYNAL, Zhang Rui, Eduardo Valentin,
devicetree, Jason Cooper, Andrew Lunn, Sebastian Hesselbarth,
Russell King, linux-pm, linux-arm-kernel
In-Reply-To: <20171213083848.j5het742yavxvkkw@sapphire.tkos.co.il>
Hi Baruch,
On mer., déc. 13 2017, Baruch Siach <baruch@tkos.co.il> wrote:
> Hi Gregory,
>
> On Mon, Dec 11, 2017 at 06:02:49PM +0100, Gregory CLEMENT wrote:
>> On lun., déc. 11 2017, Baruch Siach <baruch@tkos.co.il> wrote:
>> > On Mon, Dec 11, 2017 at 04:09:32PM +0100, Miquel RAYNAL wrote:
>> >> On Sun, 3 Dec 2017 13:11:23 +0200
>> >> Baruch Siach <baruch@tkos.co.il> wrote:
>> >>
>> >> > The CP110 component is integrated in the Armada 8k and 7k lines of
>> >> > processors.
>> >> >
>> >> > This patch also adds an option of offset to the MSB of the control
>> >> > register. The existing DT binding for Armada 38x refers to a single
>> >> > 32 bit control register. It turns out that this is actually only the
>> >> > MSB of the control area. Changing the binding to fix that would break
>> >> > existing DT files, so the Armada 38x binding is left as is.
>> >> >
>> >> > The new CP110 binding increases the size of the control area to 64
>> >> > bits, thus moving the MSB to offset 4.
>> >> >
>> >> > Signed-off-by: Baruch Siach <baruch@tkos.co.il>
>> >> > ---
>> >> > v2: No change
>> >> > ---
>> >> > drivers/thermal/armada_thermal.c | 24 ++++++++++++++++++++++--
>> >> > 1 file changed, 22 insertions(+), 2 deletions(-)
>> >> >
>> >> > diff --git a/drivers/thermal/armada_thermal.c
>> >> > b/drivers/thermal/armada_thermal.c index 0eb82097571f..59b75f63945d
>> >> > 100644 --- a/drivers/thermal/armada_thermal.c
>> >> > +++ b/drivers/thermal/armada_thermal.c
>> >> > @@ -73,6 +73,7 @@ struct armada_thermal_data {
>> >> > unsigned int temp_shift;
>> >> > unsigned int temp_mask;
>> >> > unsigned int is_valid_shift;
>> >> > + unsigned int control_msb_offset;
>> >> > };
>> >> >
>> >> > static void armadaxp_init_sensor(struct platform_device *pdev,
>> >> > @@ -142,12 +143,14 @@ static void armada375_init_sensor(struct
>> >> > platform_device *pdev, static void armada380_init_sensor(struct
>> >> > platform_device *pdev, struct armada_thermal_priv *priv)
>> >> > {
>> >> > - unsigned long reg = readl_relaxed(priv->control);
>> >> > + void __iomem *control_msb =
>> >> > + priv->control + priv->data->control_msb_offset;
>> >> > + unsigned long reg = readl_relaxed(control_msb);
>> >> >
>> >> > /* Reset hardware once */
>> >> > if (!(reg & A380_HW_RESET)) {
>> >> > reg |= A380_HW_RESET;
>> >> > - writel(reg, priv->control);
>> >> > + writel(reg, control_msb);
>> >> > mdelay(10);
>> >> > }
>> >> > }
>> >> > @@ -266,6 +269,19 @@ static const struct armada_thermal_data
>> >> > armada_ap806_data = { .signed_sample = true,
>> >> > };
>> >> >
>> >> > +static const struct armada_thermal_data armada_cp110_data = {
>> >> > + .is_valid = armada_is_valid,
>> >> > + .init_sensor = armada380_init_sensor,
>> >>
>> >> I see the initialization for CP110 thermal IP is close to
>> >> Armada-380's, but, as you point it in the commit log it is still
>> >> different.
>> >>
>> >> I don't know what is the best way to handle this but until now each
>> >> new compatible had his own ->init_sensor function, shouldn't we do
>> >> the same here as changes are requested? This would naturally avoid the
>> >> situation with Armada-380 bindings.
>> >
>> > I'm not sure I understand your suggestion.
>> >
>> > There is no difference between the CP110 and the Armada 38x, as far as I can
>> > see. The only quirk is that the existing Armada 38x DT binding is wrong I that
>> > the 'reg' property references the control MSB, while leaving the LSB
>> > out. We
>>
>> Well I would not say it was wrong but more incomplete :)
>>
>> > can't change the Armada 38x binding without breaking existing DTs. The
>> > 'control_msb_offset' field that this patch adds allows correct binding for
>> > CP110, while keeping compatibility with the existing Armada 38x
>> > binding.
>>
>> I am not against adding a new compatible string for CP110 but ot be
>> honest the new binding for CP110 does not bring anything as you don't
>> use at all the LSB register.
>
> We don't use the LSB yet in mainline driver. But the vendor kernel uses it to
> "change temperature band gap circuit curve" (quoting vendor kernel commit
> 4ff2d8a7d3 log). Chances are that we want to do this as well. But said commit
> changed the DT binding in an incompatible way. We can't do that, and we both
> agree on that.
>
>> Actually, if on Armada 375 we initially mapped the LSB register it was
>> to support an very early release of the SoC (stepping Z) and only for
>> resetting its value. So I guess you started to write the AP860 part
>> based on the Armada 375 and then found that we could map a more complete
>> range of the registers.
>>
>> > How would a separate init_sensor routine improve things?
>>
>> So yes please do it, thanks to this you won't have to add the
>> control_msb_offset member and can use a clean function. Moreover if in
>> the future we see some usefulness for this LSB register then we could use
>> the new compatible for the Armada 38x.
>
> There are two separate issues here:
>
> 1. DT binding
>
> 2. init_sensor callback implementation
>
> We both agree on #1. The A38x and CP110 need separate compatible strings. In
> case we want to access the LSB control register on Armada 38x, we will need
> yet another compatible string (marvell,armada380-v2-thermal maybe?).
Actually, if it is _compatible_ then we will use the same compatible, ie
"marvell,armadacp110-thermal"
>
> As for #2, I'm all for sharing as much code as possible. I find the vendor
> kernel approach of duplicating the init routines[1] unhelpful as it violates
> the DRY principle. The differences between armada380_init_sensor() and
> cp110_init_sensor() are minor. In my opinion, these differences should be
> expressed explicitly in the armada_thermal_data, in a similar way to my
> suggested control_msb_offset field. The vendor code hides these differences in
> slight variations of duplicated code.
>
> What is the advantage of a separate init routine?
The main advantage is to be able keep the armada380_init_sensor as the
legacy init, and then being able to use the new armadacp110_init_sensor
for the new binding.
Gregory
>
> baruch
>
> [1] https://github.com/MarvellEmbeddedProcessors/linux-marvell/blob/linux-4.4.52-armada-17.10/drivers/thermal/armada_thermal.c
>
> --
> http://baruch.siach.name/blog/ ~. .~ Tk Open Systems
> =}------------------------------------------------ooO--U--Ooo------------{=
> - baruch@tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -
--
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
^ permalink raw reply
* Re: [PATCH v2 3/4] thermal: armada: add support for CP110
From: Baruch Siach @ 2017-12-13 9:10 UTC (permalink / raw)
To: Miquel RAYNAL
Cc: Gregory CLEMENT, Ezequiel Garcia, Zhang Rui, Eduardo Valentin,
devicetree-u79uwXL29TY76Z2rM5mHXA, Jason Cooper, Andrew Lunn,
Sebastian Hesselbarth, Russell King,
linux-pm-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <20171213095501.1988749d@xps13>
Hi Miquel,
On Wed, Dec 13, 2017 at 09:55:01AM +0100, Miquel RAYNAL wrote:
> > > > How would a separate init_sensor routine improve things?
> > >
> > > So yes please do it, thanks to this you won't have to add the
> > > control_msb_offset member and can use a clean function. Moreover if
> > > in the future we see some usefulness for this LSB register then we
> > > could use the new compatible for the Armada 38x.
> >
> > There are two separate issues here:
> >
> > 1. DT binding
> >
> > 2. init_sensor callback implementation
> >
> > We both agree on #1. The A38x and CP110 need separate compatible
> > strings. In case we want to access the LSB control register on Armada
> > 38x, we will need yet another compatible string
> > (marvell,armada380-v2-thermal maybe?).
> >
> > As for #2, I'm all for sharing as much code as possible. I find the
> > vendor kernel approach of duplicating the init routines[1] unhelpful
> > as it violates the DRY principle. The differences between
> > armada380_init_sensor() and cp110_init_sensor() are minor. In my
> > opinion, these differences should be expressed explicitly in the
> > armada_thermal_data, in a similar way to my suggested
> > control_msb_offset field. The vendor code hides these differences in
> > slight variations of duplicated code.
> >
> > What is the advantage of a separate init routine?
>
> The advantage is that is the very near future I plan to add the
> overheat interrupt only on CP110 (not on 38x) and this needs some
> initialization. So if we don't make different routines now, I will
> have to do it right after.
I don't think so. The code of these functions in the vendor kernel overheat
support implementation is the same, duplicated. The variations are only in
registers/bits offsets. A single routine with one or two added
armada_thermal_data fields would be much easier to comprehend and maintain.
> What would be fine is to have the shared code in a separate function,
> like it is done in Marvell kernel. What do you think about that?
The Marvell code does not "share" the code. Separate functions means
duplicated code that obscures the hardware details, making maintenance harder
on the long run.
https://en.wikipedia.org/wiki/Don%27t_repeat_yourself
baruch
--
http://baruch.siach.name/blog/ ~. .~ Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
- baruch-NswTu9S1W3P6gbPvEgmw2w@public.gmane.org - tel: +972.2.679.5364, http://www.tkos.co.il -
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 2/5] phy: renesas: rcar-gen3-usb2: unify OBINTEN handling
From: Sergei Shtylyov @ 2017-12-13 9:09 UTC (permalink / raw)
To: Yoshihiro Shimoda, kishon-l0cyMroinI0,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-renesas-soc-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1513146460-20326-3-git-send-email-yoshihiro.shimoda.uh-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
Hello!
On 12/13/2017 9:27 AM, Yoshihiro Shimoda wrote:
> This patch unifies the OBINTEN handling to clean-up the code.
>
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
> ---
> drivers/phy/renesas/phy-rcar-gen3-usb2.c | 23 +++++++++++++++--------
> 1 file changed, 15 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/phy/renesas/phy-rcar-gen3-usb2.c b/drivers/phy/renesas/phy-rcar-gen3-usb2.c
> index c22d65a..beeaa30 100644
> --- a/drivers/phy/renesas/phy-rcar-gen3-usb2.c
> +++ b/drivers/phy/renesas/phy-rcar-gen3-usb2.c
> @@ -147,6 +147,18 @@ static void rcar_gen3_enable_vbus_ctrl(struct rcar_gen3_chan *ch, int vbus)
> writel(val, usb2_base + USB2_ADPCTRL);
> }
>
> +static void rcar_gen3_enable_otg_irq(struct rcar_gen3_chan *ch, int enable)
If it both enables and disables, rcar_gen3_control_otg_irq() would seem a
better name...
> +{
> + void __iomem *usb2_base = ch->base;
> + u32 val = readl(usb2_base + USB2_OBINTEN);
> +
> + if (enable)
> + val |= USB2_OBINT_BITS;
> + else
> + val &= ~USB2_OBINT_BITS;
> + writel(val, usb2_base + USB2_OBINTEN);
> +}
> +
> static void rcar_gen3_init_for_host(struct rcar_gen3_chan *ch)
> {
> rcar_gen3_set_linectrl(ch, 1, 1);
[...]
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 v3 09/33] nds32: Cache and TLB routines
From: Greentime Hu @ 2017-12-13 9:03 UTC (permalink / raw)
To: Guo Ren
Cc: Greentime, Linux Kernel Mailing List, Arnd Bergmann, linux-arch,
Thomas Gleixner, Jason Cooper, Marc Zyngier, Rob Herring, netdev,
Vincent Chen, DTML, Al Viro, David Howells, Will Deacon,
Daniel Lezcano, linux-serial, Geert Uytterhoeven, Linus Walleij,
Mark Rutland, Greg KH
In-Reply-To: <20171213085334.GA21382@gary-OptiPlex-3050>
2017-12-13 16:53 GMT+08:00 Guo Ren <ren_guo@c-sky.com>:
> On Wed, Dec 13, 2017 at 04:30:41PM +0800, Greentime Hu wrote:
>> 2017-12-13 16:19 GMT+08:00 Guo Ren <ren_guo@c-sky.com>:
>> > On Wed, Dec 13, 2017 at 01:45:02PM +0800, Greentime Hu wrote:
>> >
>> >> I think it should be fine if an interruption between mtsr_dsb and
>> >> tlbop_rwr because this is a optimization by sw.
>> >
>> > Fine? When there is an unexpected vaddr in SR_TLB_VPN, tlbop_rwr(*pte) will
>> > break that vaddr's pfn in the CPU tlb-buffer entry. When linux access the
>> > vaddr, it will get wrong data unless the entry has been replaced out.
>>
>> Hi, Guo Ren:
>>
>> Thanks. I get your point.
>> It is needed to be protected.
>> I will fix it in the next version patch.
>>
>> if (vma->vm_mm == current->active_mm) {
>> local_irq_save(flags);
>> __nds32__mtsr_dsb(addr, NDS32_SR_TLB_VPN);
>> __nds32__tlbop_rwr(*pte);
>> __nds32__isb();
>> local_irq_restore(flags);
>> }
>
> If hardware tlbop_rwr could invalid NDS32_SR_TLB_VPN, then you needn't
> protect.
> I mean:
> mtsr addr1 NDS32_SR_TLB_VPN
> mtsr addr2 NDS32_SR_TLB_VPN
> tlbop_rwr(*pte) // OK, and it will hit a hardware invalid bit internal.
> tlbop_rwr(*pte) // SR_TLB_VPN invalided, then it will not cause problem.
>
> :) How my idea?
Hi, Guo Ren:
The reason I think it might have problem is that
mtsr addr1 NDS32_SR_TLB_VPN
interrupt coming
mtsr addr2 NDS32_SR_TLB_VPN <- TLB_VPN has been set to addr2
tlbop_rwr(*pte);
interrupt finish
tlbop_rwr(*pte); <- it will use the wrong TLB_VPN
I think all these TLB operations should be protected because tlbop
will operate with TLB_VPN.
Thanks :)
^ permalink raw reply
* Re: [PATCH 2/6] ARM: stm32: add initial support for STM32MP157
From: Ludovic BARRE @ 2017-12-13 9:02 UTC (permalink / raw)
To: Rob Herring
Cc: Russell King, Linus Walleij, Arnd Bergmann, Maxime Coquelin,
Alexandre Torgue, linux-arm-kernel, linux-kernel, devicetree
In-Reply-To: <20171212232425.shf56g2uivl57fdp@rob-hp-laptop>
On 12/13/2017 12:24 AM, Rob Herring wrote:
> On Fri, Dec 08, 2017 at 03:11:13PM +0100, Ludovic Barre wrote:
>> From: Ludovic Barre <ludovic.barre@st.com>
>>
>> This patch adds initial support of STM32MP157 microprocessor (MPU)
>> based on Arm Cortex-A7. Under new ARCH_STM32_MPU flag we select the
>> needed Cortex-A infrastructure (like gic, timer,...)
>>
>> Signed-off-by: Ludovic Barre <ludovic.barre@st.com>
>> ---
>> Documentation/arm/stm32/stm32mp157-overview.txt | 12 ++++++++++++
>> Documentation/devicetree/bindings/arm/stm32.txt | 1 +
>
> Please split bindings to separate patches.
OK, I will split stm32.txt in separate commit
>
>> arch/arm/mach-stm32/Kconfig | 22 ++++++++++++++++++++--
>> arch/arm/mach-stm32/Makefile | 1 +
>> arch/arm/mach-stm32/board-mpu-dt.c | 16 ++++++++++++++++
>> 5 files changed, 50 insertions(+), 2 deletions(-)
>> create mode 100644 Documentation/arm/stm32/stm32mp157-overview.txt
>> create mode 100644 arch/arm/mach-stm32/board-mpu-dt.c
>>
>> diff --git a/Documentation/arm/stm32/stm32mp157-overview.txt b/Documentation/arm/stm32/stm32mp157-overview.txt
>> new file mode 100644
>> index 0000000..8a3e7cb
>> --- /dev/null
>> +++ b/Documentation/arm/stm32/stm32mp157-overview.txt
>
> I think new documentation files should be rst format and fit into the
> built documentation. We don't have an SoC description doc for most SoCs.
the existing documentation of stm32 are txt format
-overview.txt
-stm32f429-overwiew.txt
-stm32f746-overview.txt
-stm32h743-overview.txt
what do you prefer:
-omit stm32mp157-overview.txt of this serie and change all in next commit.
-write only this file in rst format
-change all in this serie?
>
>> @@ -0,0 +1,12 @@
>> + STM32MP157 Overview
>> + ===================
>> +
>> + Introduction
>> + ------------
>> + The STM32MP157 is a Cortex-A MPU aimed at various applications.
>> + It features:
>> + - Dual core Cortex-A7 application core
>> + - 2D/3D image composition with GPU
>> + - Standard memories interface support
>> + - Standard connectivity, widely inherited from the STM32 MCU family
>> + - Comprehensive security support
>
> Perhaps make this part of the kconfig entry help.
yes, I could add some details in MACH_STM32MP157 kconfig entry
>
>> diff --git a/Documentation/devicetree/bindings/arm/stm32.txt b/Documentation/devicetree/bindings/arm/stm32.txt
>> index 05762b0..6808ed9 100644
>> --- a/Documentation/devicetree/bindings/arm/stm32.txt
>> +++ b/Documentation/devicetree/bindings/arm/stm32.txt
>> @@ -7,3 +7,4 @@ using one of the following compatible strings:
>> st,stm32f469
>> st,stm32f746
>> st,stm32h743
>> + st,stm32mp157
>> diff --git a/arch/arm/mach-stm32/Kconfig b/arch/arm/mach-stm32/Kconfig
>> index c8059ea..2b227c7 100644
>> --- a/arch/arm/mach-stm32/Kconfig
>> +++ b/arch/arm/mach-stm32/Kconfig
>> @@ -1,12 +1,12 @@
>> menuconfig ARCH_STM32
>> - bool "STMicrolectronics STM32 family" if ARM_SINGLE_ARMV7M
>> + bool "STMicrolectronics STM32 family" if ARM_SINGLE_ARMV7M || ARCH_MULTI_V7
>> select ARCH_HAS_RESET_CONTROLLER
>> select CLKSRC_STM32
>> select PINCTRL
>> select RESET_CONTROLLER
>> select STM32_EXTI
>> help
>> - Support for STMicroelectronics STM32 MCU family
>> + Support for STMicroelectronics STM32 MCU/MPU family
>>
>> if ARCH_STM32
>>
>> @@ -40,4 +40,22 @@ config MACH_STM32H743
>>
>> endif
>>
>> +if ARCH_MULTI_V7
>> +
>> +config ARCH_STM32_MPU
>> + bool "STMicrolectronics STM32 MPU"
>> + default y
>> + select ARM_GIC
>> + select HAVE_ARM_ARCH_TIMER
>> + select ARM_PSCI
>> + help
>> + Support for STMicroelectronics STM32 Microprocessors.
>> +
>> +config MACH_STM32MP157
>
> Is this actually used?
Yes, it's used in pinctrl driver.
>
>> + bool "STMicrolectronics STM32MP157"
>> + depends on ARCH_STM32_MPU
>> + default y
>> +
>> +endif
>> +
>> endif
^ permalink raw reply
* Re: [PATCH 0/5] Add Sound support for iWave RZ/G1M board
From: Simon Horman @ 2017-12-13 9:02 UTC (permalink / raw)
To: Biju Das
Cc: Rob Herring, Mark Rutland, Russell King, Magnus Damm,
Chris Paterson, devicetree, linux-renesas-soc, linux-arm-kernel
In-Reply-To: <1513103111-45830-1-git-send-email-biju.das@bp.renesas.com>
On Tue, Dec 12, 2017 at 06:25:06PM +0000, Biju Das wrote:
> This series aims to add sound support for iWave RZ/G1M board.
>
> This patch series has documentation dependency on
> https://patchwork.kernel.org/patch/10108014/
>
> Biju Das (5):
> ARM: shmobile: defconfig: Enable SGTL5000 audio codec
> ARM: dts: r8a7743: Add audio clocks
> ARM: dts: r8a7743: Add audio DMAC support
> ARM: dts: r8a7743: Add sound support
> ARM: dts: iwg20d-q7-common: Enable SGTL5000 audio codec
>
> arch/arm/boot/dts/iwg20d-q7-common.dtsi | 24 +++
> arch/arm/boot/dts/r8a7743.dtsi | 270 ++++++++++++++++++++++++++++++++
> arch/arm/configs/shmobile_defconfig | 1 +
> 3 files changed, 295 insertions(+)
These patches seem clean to me although I do not have sufficient
documentation to properly review the last patch.
I will leave these sit for a few days to allow others to review them.
^ permalink raw reply
* Re: [PATCH 5/5] phy: renesas: rcar-gen3-usb2: add gpio handling
From: Geert Uytterhoeven @ 2017-12-13 8:55 UTC (permalink / raw)
To: Yoshihiro Shimoda
Cc: Kishon Vijay Abraham I, Rob Herring, Mark Rutland,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, Linux-Renesas
In-Reply-To: <1513146460-20326-6-git-send-email-yoshihiro.shimoda.uh-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
Hi Shimoda-san,
On Wed, Dec 13, 2017 at 7:27 AM, Yoshihiro Shimoda
<yoshihiro.shimoda.uh-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org> wrote:
> Some R-Car SoCs (e.g. R-Car D3) doesn't have dedicated pins of VBUS
> and ID. So, they may be connected to gpio pins. To handle the gpio
> pins, this patch adds the handling of VBUS and ID pins instead of
> dedicated pins.
>
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
> ---
> .../devicetree/bindings/phy/rcar-gen3-phy-usb2.txt | 2 +
> drivers/phy/renesas/phy-rcar-gen3-usb2.c | 77 ++++++++++++++++++++--
> 2 files changed, 72 insertions(+), 7 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/phy/rcar-gen3-phy-usb2.txt b/Documentation/devicetree/bindings/phy/rcar-gen3-phy-usb2.txt
> index 99b651b..851582f 100644
> --- a/Documentation/devicetree/bindings/phy/rcar-gen3-phy-usb2.txt
> +++ b/Documentation/devicetree/bindings/phy/rcar-gen3-phy-usb2.txt
> @@ -27,6 +27,8 @@ channel as USB OTG:
> - interrupts: interrupt specifier for the PHY.
> - vbus-supply: Phandle to a regulator that provides power to the VBUS. This
> regulator will be managed during the PHY power on/off sequence.
> +- renesas,vbus-gpios: use gpio to control vbus instead of dedicated pin.
> +- renesas,id-gpios: use gpio to detect id instead of dedicated pin.
Documentation/devicetree/bindings/phy/brcm,ns2-drd-phy.txt already uses
"vbus-gpios" and "id-gpios" without vendor-specific prefixes, so perhaps
the "renesas," can be dropped?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.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
--
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 v2 3/4] thermal: armada: add support for CP110
From: Miquel RAYNAL @ 2017-12-13 8:55 UTC (permalink / raw)
To: Baruch Siach
Cc: Gregory CLEMENT, Ezequiel Garcia, Zhang Rui, Eduardo Valentin,
devicetree, Jason Cooper, Andrew Lunn, Sebastian Hesselbarth,
Russell King, linux-pm, linux-arm-kernel
In-Reply-To: <20171213083848.j5het742yavxvkkw@sapphire.tkos.co.il>
Hello Baruch,
> > > How would a separate init_sensor routine improve things?
> >
> > So yes please do it, thanks to this you won't have to add the
> > control_msb_offset member and can use a clean function. Moreover if
> > in the future we see some usefulness for this LSB register then we
> > could use the new compatible for the Armada 38x.
>
> There are two separate issues here:
>
> 1. DT binding
>
> 2. init_sensor callback implementation
>
> We both agree on #1. The A38x and CP110 need separate compatible
> strings. In case we want to access the LSB control register on Armada
> 38x, we will need yet another compatible string
> (marvell,armada380-v2-thermal maybe?).
>
> As for #2, I'm all for sharing as much code as possible. I find the
> vendor kernel approach of duplicating the init routines[1] unhelpful
> as it violates the DRY principle. The differences between
> armada380_init_sensor() and cp110_init_sensor() are minor. In my
> opinion, these differences should be expressed explicitly in the
> armada_thermal_data, in a similar way to my suggested
> control_msb_offset field. The vendor code hides these differences in
> slight variations of duplicated code.
>
> What is the advantage of a separate init routine?
The advantage is that is the very near future I plan to add the
overheat interrupt only on CP110 (not on 38x) and this needs some
initialization. So if we don't make different routines now, I will
have to do it right after.
What would be fine is to have the shared code in a separate function,
like it is done in Marvell kernel. What do you think about that?
Thanks,
Miquèl
^ permalink raw reply
* Re: [PATCH v3 09/33] nds32: Cache and TLB routines
From: Guo Ren @ 2017-12-13 8:53 UTC (permalink / raw)
To: Greentime Hu
Cc: Greentime, Linux Kernel Mailing List, Arnd Bergmann, linux-arch,
Thomas Gleixner, Jason Cooper, Marc Zyngier, Rob Herring, netdev,
Vincent Chen, DTML, Al Viro, David Howells, Will Deacon,
Daniel Lezcano, linux-serial-u79uwXL29TY76Z2rM5mHXA,
Geert Uytterhoeven, Linus Walleij, Mark Rutland, Greg KH
In-Reply-To: <CAEbi=3eKH5JESwtadq4LKF3ZvmBi1QwUWpCTb0btierBST_cRQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Wed, Dec 13, 2017 at 04:30:41PM +0800, Greentime Hu wrote:
> 2017-12-13 16:19 GMT+08:00 Guo Ren <ren_guo-Y+KPrCd2zL4AvxtiuMwx3w@public.gmane.org>:
> > On Wed, Dec 13, 2017 at 01:45:02PM +0800, Greentime Hu wrote:
> >
> >> I think it should be fine if an interruption between mtsr_dsb and
> >> tlbop_rwr because this is a optimization by sw.
> >
> > Fine? When there is an unexpected vaddr in SR_TLB_VPN, tlbop_rwr(*pte) will
> > break that vaddr's pfn in the CPU tlb-buffer entry. When linux access the
> > vaddr, it will get wrong data unless the entry has been replaced out.
>
> Hi, Guo Ren:
>
> Thanks. I get your point.
> It is needed to be protected.
> I will fix it in the next version patch.
>
> if (vma->vm_mm == current->active_mm) {
> local_irq_save(flags);
> __nds32__mtsr_dsb(addr, NDS32_SR_TLB_VPN);
> __nds32__tlbop_rwr(*pte);
> __nds32__isb();
> local_irq_restore(flags);
> }
If hardware tlbop_rwr could invalid NDS32_SR_TLB_VPN, then you needn't
protect.
I mean:
mtsr addr1 NDS32_SR_TLB_VPN
mtsr addr2 NDS32_SR_TLB_VPN
tlbop_rwr(*pte) // OK, and it will hit a hardware invalid bit internal.
tlbop_rwr(*pte) // SR_TLB_VPN invalided, then it will not cause problem.
:) How my idea?
--
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 V8 0/7] dmaengine: qcom_hidma: add support for bugfixed HW
From: Vinod Koul @ 2017-12-13 8:43 UTC (permalink / raw)
To: Sinan Kaya
Cc: dmaengine, timur, devicetree, linux-acpi, sakari.ailus,
linux-arm-msm, linux-arm-kernel
In-Reply-To: <1513149653-19451-1-git-send-email-okaya@codeaurora.org>
On Wed, Dec 13, 2017 at 02:20:46AM -0500, Sinan Kaya wrote:
> Introduce new ACPI and OF device ids for thw HW along with the helper
> functions.
Applied, thanks
--
~Vinod
^ permalink raw reply
* Re: [PATCH 3/3] ARM: dts: r8a7745: Add CMT SoC specific support
From: Simon Horman @ 2017-12-13 8:42 UTC (permalink / raw)
To: Fabrizio Castro
Cc: Rob Herring, Mark Rutland, Russell King, Magnus Damm,
Geert Uytterhoeven, Chris Paterson, Biju Das, linux-renesas-soc,
devicetree, linux-arm-kernel
In-Reply-To: <1513104579-6333-4-git-send-email-fabrizio.castro@bp.renesas.com>
On Tue, Dec 12, 2017 at 06:49:39PM +0000, Fabrizio Castro wrote:
> Add CMT[01] support to SoC DT.
>
> Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> Reviewed-by: Biju Das <biju.das@bp.renesas.com>
> ---
> arch/arm/boot/dts/r8a7745.dtsi | 30 ++++++++++++++++++++++++++++++
> 1 file changed, 30 insertions(+)
Please see my review of the r8a7743 patch in this series.
^ permalink raw reply
* Re: [PATCH v8 10/13] IIO: ADC: add stm32 DFSDM support for PDM microphone
From: Arnaud Pouliquen @ 2017-12-13 8:42 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Mark Rutland, devicetree, alsa-devel, Lars-Peter Clausen,
Maxime Coquelin, Liam Girdwood, linux-iio, Mark Brown,
Takashi Iwai, Rob Herring, Peter Meerwald-Stadler, Hartmut Knaack,
linux-arm-kernel, Alexandre Torgue
In-Reply-To: <20171212202715.2d9bd9f2@archlinux>
Hi Jonathan,
On 12/12/2017 09:27 PM, Jonathan Cameron wrote:
> On Mon, 11 Dec 2017 11:18:41 +0100
> Arnaud Pouliquen <arnaud.pouliquen@st.com> wrote:
>
>> This code offers a way to handle PDM audio microphones in
>> ASOC framework. Audio driver should use consumer API.
>> A specific management is implemented for DMA, with a
>> callback, to allows to handle audio buffers efficiently.
>>
>> Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@st.com>
> Hi Arnaud,
>
> I raise a few queries on v7 of this patch.
>
> https://marc.info/?l=linux-iio&m=151292965915376&w=2
Never received the associated mail (and no in my spam list):( ,I just
discover it...
Thanks to have highlighted this and sorry for the inconvenience, I will
send a v9.
Regards
Arnaud
>
> Jonathan
>
>> ---
>> .../ABI/testing/sysfs-bus-iio-dfsdm-adc-stm32 | 16 +
>> drivers/iio/adc/stm32-dfsdm-adc.c | 508 ++++++++++++++++++++-
>> include/linux/iio/adc/stm32-dfsdm-adc.h | 18 +
>> 3 files changed, 534 insertions(+), 8 deletions(-)
>> create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-dfsdm-adc-stm32
>> create mode 100644 include/linux/iio/adc/stm32-dfsdm-adc.h
>>
>> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-dfsdm-adc-stm32 b/Documentation/ABI/testing/sysfs-bus-iio-dfsdm-adc-stm32
>> new file mode 100644
>> index 0000000..da98223
>> --- /dev/null
>> +++ b/Documentation/ABI/testing/sysfs-bus-iio-dfsdm-adc-stm32
>> @@ -0,0 +1,16 @@
>> +What: /sys/bus/iio/devices/iio:deviceX/in_voltage_spi_clk_freq
>> +KernelVersion: 4.14
>> +Contact: arnaud.pouliquen@st.com
>> +Description:
>> + For audio purpose only.
>> + Used by audio driver to set/get the spi input frequency.
>> + This is mandatory if DFSDM is slave on SPI bus, to
>> + provide information on the SPI clock frequency during runtime
>> + Notice that the SPI frequency should be a multiple of sample
>> + frequency to ensure the precision.
>> + if DFSDM input is SPI master
>> + Reading SPI clkout frequency,
>> + error on writing
>> + If DFSDM input is SPI Slave:
>> + Reading returns value previously set.
>> + Writing value before starting conversions.
>> \ No newline at end of file
>> diff --git a/drivers/iio/adc/stm32-dfsdm-adc.c b/drivers/iio/adc/stm32-dfsdm-adc.c
>> index 68b5920..2d6aed5 100644
>> --- a/drivers/iio/adc/stm32-dfsdm-adc.c
>> +++ b/drivers/iio/adc/stm32-dfsdm-adc.c
>> @@ -6,19 +6,25 @@
>> * Author: Arnaud Pouliquen <arnaud.pouliquen@st.com>.
>> */
>>
>> +#include <linux/dmaengine.h>
>> +#include <linux/dma-mapping.h>
>> #include <linux/interrupt.h>
>> #include <linux/iio/buffer.h>
>> #include <linux/iio/hw-consumer.h>
>> #include <linux/iio/iio.h>
>> #include <linux/iio/sysfs.h>
>> +#include <linux/iio/trigger_consumer.h>
>> +#include <linux/iio/triggered_buffer.h>
>> #include <linux/module.h>
>> -#include <linux/of.h>
>> +#include <linux/of_device.h>
>> #include <linux/platform_device.h>
>> #include <linux/regmap.h>
>> #include <linux/slab.h>
>>
>> #include "stm32-dfsdm.h"
>>
>> +#define DFSDM_DMA_BUFFER_SIZE (4 * PAGE_SIZE)
>> +
>> /* Conversion timeout */
>> #define DFSDM_TIMEOUT_US 100000
>> #define DFSDM_TIMEOUT (msecs_to_jiffies(DFSDM_TIMEOUT_US / 1000))
>> @@ -58,6 +64,18 @@ struct stm32_dfsdm_adc {
>> struct completion completion;
>> u32 *buffer;
>>
>> + /* Audio specific */
>> + unsigned int spi_freq; /* SPI bus clock frequency */
>> + unsigned int sample_freq; /* Sample frequency after filter decimation */
>> + int (*cb)(const void *data, size_t size, void *cb_priv);
>> + void *cb_priv;
>> +
>> + /* DMA */
>> + u8 *rx_buf;
>> + unsigned int bufi; /* Buffer current position */
>> + unsigned int buf_sz; /* Buffer size */
>> + struct dma_chan *dma_chan;
>> + dma_addr_t dma_buf;
>> };
>>
>> struct stm32_dfsdm_str2field {
>> @@ -351,10 +369,63 @@ int stm32_dfsdm_channel_parse_of(struct stm32_dfsdm *dfsdm,
>> return 0;
>> }
>>
>> +static ssize_t dfsdm_adc_audio_get_spiclk(struct iio_dev *indio_dev,
>> + uintptr_t priv,
>> + const struct iio_chan_spec *chan,
>> + char *buf)
>> +{
>> + struct stm32_dfsdm_adc *adc = iio_priv(indio_dev);
>> +
>> + return snprintf(buf, PAGE_SIZE, "%d\n", adc->spi_freq);
>> +}
>> +
>> +static ssize_t dfsdm_adc_audio_set_spiclk(struct iio_dev *indio_dev,
>> + uintptr_t priv,
>> + const struct iio_chan_spec *chan,
>> + const char *buf, size_t len)
>> +{
>> + struct stm32_dfsdm_adc *adc = iio_priv(indio_dev);
>> + struct stm32_dfsdm_filter *fl = &adc->dfsdm->fl_list[adc->fl_id];
>> + struct stm32_dfsdm_channel *ch = &adc->dfsdm->ch_list[adc->ch_id];
>> + unsigned int sample_freq = adc->sample_freq;
>> + unsigned int spi_freq;
>> + int ret;
>> +
>> + dev_err(&indio_dev->dev, "enter %s\n", __func__);
>> + /* If DFSDM is master on SPI, SPI freq can not be updated */
>> + if (ch->src != DFSDM_CHANNEL_SPI_CLOCK_EXTERNAL)
>> + return -EPERM;
>> +
>> + ret = kstrtoint(buf, 0, &spi_freq);
>> + if (ret)
>> + return ret;
>> +
>> + if (!spi_freq)
>> + return -EINVAL;
>> +
>> + if (sample_freq) {
>> + if (spi_freq % sample_freq)
>> + dev_warn(&indio_dev->dev,
>> + "Sampling rate not accurate (%d)\n",
>> + spi_freq / (spi_freq / sample_freq));
>> +
>> + ret = stm32_dfsdm_set_osrs(fl, 0, (spi_freq / sample_freq));
>> + if (ret < 0) {
>> + dev_err(&indio_dev->dev,
>> + "No filter parameters that match!\n");
>> + return ret;
>> + }
>> + }
>> + adc->spi_freq = spi_freq;
>> +
>> + return len;
>> +}
>> +
>> static int stm32_dfsdm_start_conv(struct stm32_dfsdm_adc *adc, bool dma)
>> {
>> struct regmap *regmap = adc->dfsdm->regmap;
>> int ret;
>> + unsigned int dma_en = 0, cont_en = 0;
>>
>> ret = stm32_dfsdm_start_channel(adc->dfsdm, adc->ch_id);
>> if (ret < 0)
>> @@ -365,6 +436,24 @@ static int stm32_dfsdm_start_conv(struct stm32_dfsdm_adc *adc, bool dma)
>> if (ret < 0)
>> goto stop_channels;
>>
>> + if (dma) {
>> + /* Enable DMA transfer*/
>> + dma_en = DFSDM_CR1_RDMAEN(1);
>> + /* Enable conversion triggered by SPI clock*/
>> + cont_en = DFSDM_CR1_RCONT(1);
>> + }
>> + /* Enable DMA transfer*/
>> + ret = regmap_update_bits(regmap, DFSDM_CR1(adc->fl_id),
>> + DFSDM_CR1_RDMAEN_MASK, dma_en);
>> + if (ret < 0)
>> + goto stop_channels;
>> +
>> + /* Enable conversion triggered by SPI clock*/
>> + ret = regmap_update_bits(regmap, DFSDM_CR1(adc->fl_id),
>> + DFSDM_CR1_RCONT_MASK, cont_en);
>> + if (ret < 0)
>> + goto stop_channels;
>> +
>> ret = stm32_dfsdm_start_filter(adc->dfsdm, adc->fl_id);
>> if (ret < 0)
>> goto stop_channels;
>> @@ -398,6 +487,231 @@ static void stm32_dfsdm_stop_conv(struct stm32_dfsdm_adc *adc)
>> stm32_dfsdm_stop_channel(adc->dfsdm, adc->ch_id);
>> }
>>
>> +static int stm32_dfsdm_set_watermark(struct iio_dev *indio_dev,
>> + unsigned int val)
>> +{
>> + struct stm32_dfsdm_adc *adc = iio_priv(indio_dev);
>> + unsigned int watermark = DFSDM_DMA_BUFFER_SIZE / 2;
>> +
>> + /*
>> + * DMA cyclic transfers are used, buffer is split into two periods.
>> + * There should be :
>> + * - always one buffer (period) DMA is working on
>> + * - one buffer (period) driver pushed to ASoC side.
>> + */
>> + watermark = min(watermark, val * (unsigned int)(sizeof(u32)));
>> + adc->buf_sz = watermark * 2;
>> +
>> + return 0;
>> +}
>> +
>> +static unsigned int stm32_dfsdm_adc_dma_residue(struct stm32_dfsdm_adc *adc)
>> +{
>> + struct dma_tx_state state;
>> + enum dma_status status;
>> +
>> + status = dmaengine_tx_status(adc->dma_chan,
>> + adc->dma_chan->cookie,
>> + &state);
>> + if (status == DMA_IN_PROGRESS) {
>> + /* Residue is size in bytes from end of buffer */
>> + unsigned int i = adc->buf_sz - state.residue;
>> + unsigned int size;
>> +
>> + /* Return available bytes */
>> + if (i >= adc->bufi)
>> + size = i - adc->bufi;
>> + else
>> + size = adc->buf_sz + i - adc->bufi;
>> +
>> + return size;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static void stm32_dfsdm_audio_dma_buffer_done(void *data)
>> +{
>> + struct iio_dev *indio_dev = data;
>> + struct stm32_dfsdm_adc *adc = iio_priv(indio_dev);
>> + int available = stm32_dfsdm_adc_dma_residue(adc);
>> + size_t old_pos;
>> +
>> + /*
>> + * FIXME: In Kernel interface does not support cyclic DMA buffer,and
>> + * offers only an interface to push data samples per samples.
>> + * For this reason IIO buffer interface is not used and interface is
>> + * bypassed using a private callback registered by ASoC.
>> + * This should be a temporary solution waiting a cyclic DMA engine
>> + * support in IIO.
>> + */
>> +
>> + dev_dbg(&indio_dev->dev, "%s: pos = %d, available = %d\n", __func__,
>> + adc->bufi, available);
>> + old_pos = adc->bufi;
>> +
>> + while (available >= indio_dev->scan_bytes) {
>> + u32 *buffer = (u32 *)&adc->rx_buf[adc->bufi];
>> +
>> + /* Mask 8 LSB that contains the channel ID */
>> + *buffer = (*buffer & 0xFFFFFF00) << 8;
>> + available -= indio_dev->scan_bytes;
>> + adc->bufi += indio_dev->scan_bytes;
>> + if (adc->bufi >= adc->buf_sz) {
>> + if (adc->cb)
>> + adc->cb(&adc->rx_buf[old_pos],
>> + adc->buf_sz - old_pos, adc->cb_priv);
>> + adc->bufi = 0;
>> + old_pos = 0;
>> + }
>> + }
>> + if (adc->cb)
>> + adc->cb(&adc->rx_buf[old_pos], adc->bufi - old_pos,
>> + adc->cb_priv);
>> +}
>> +
>> +static int stm32_dfsdm_adc_dma_start(struct iio_dev *indio_dev)
>> +{
>> + struct stm32_dfsdm_adc *adc = iio_priv(indio_dev);
>> + struct dma_async_tx_descriptor *desc;
>> + dma_cookie_t cookie;
>> + int ret;
>> +
>> + if (!adc->dma_chan)
>> + return -EINVAL;
>> +
>> + dev_dbg(&indio_dev->dev, "%s size=%d watermark=%d\n", __func__,
>> + adc->buf_sz, adc->buf_sz / 2);
>> +
>> + /* Prepare a DMA cyclic transaction */
>> + desc = dmaengine_prep_dma_cyclic(adc->dma_chan,
>> + adc->dma_buf,
>> + adc->buf_sz, adc->buf_sz / 2,
>> + DMA_DEV_TO_MEM,
>> + DMA_PREP_INTERRUPT);
>> + if (!desc)
>> + return -EBUSY;
>> +
>> + desc->callback = stm32_dfsdm_audio_dma_buffer_done;
>> + desc->callback_param = indio_dev;
>> +
>> + cookie = dmaengine_submit(desc);
>> + ret = dma_submit_error(cookie);
>> + if (ret) {
>> + dmaengine_terminate_all(adc->dma_chan);
>> + return ret;
>> + }
>> +
>> + /* Issue pending DMA requests */
>> + dma_async_issue_pending(adc->dma_chan);
>> +
>> + return 0;
>> +}
>> +
>> +static int stm32_dfsdm_postenable(struct iio_dev *indio_dev)
>> +{
>> + struct stm32_dfsdm_adc *adc = iio_priv(indio_dev);
>> + int ret;
>> +
>> + /* Reset adc buffer index */
>> + adc->bufi = 0;
>> +
>> + ret = stm32_dfsdm_start_dfsdm(adc->dfsdm);
>> + if (ret < 0)
>> + return ret;
>> +
>> + ret = stm32_dfsdm_start_conv(adc, true);
>> + if (ret) {
>> + dev_err(&indio_dev->dev, "Can't start conversion\n");
>> + goto stop_dfsdm;
>> + }
>> +
>> + if (adc->dma_chan) {
>> + ret = stm32_dfsdm_adc_dma_start(indio_dev);
>> + if (ret) {
>> + dev_err(&indio_dev->dev, "Can't start DMA\n");
>> + goto err_stop_conv;
>> + }
>> + }
>> +
>> + return 0;
>> +
>> +err_stop_conv:
>> + stm32_dfsdm_stop_conv(adc);
>> +stop_dfsdm:
>> + stm32_dfsdm_stop_dfsdm(adc->dfsdm);
>> +
>> + return ret;
>> +}
>> +
>> +static int stm32_dfsdm_predisable(struct iio_dev *indio_dev)
>> +{
>> + struct stm32_dfsdm_adc *adc = iio_priv(indio_dev);
>> +
>> + if (adc->dma_chan)
>> + dmaengine_terminate_all(adc->dma_chan);
>> +
>> + stm32_dfsdm_stop_conv(adc);
>> +
>> + stm32_dfsdm_stop_dfsdm(adc->dfsdm);
>> +
>> + return 0;
>> +}
>> +
>> +static const struct iio_buffer_setup_ops stm32_dfsdm_buffer_setup_ops = {
>> + .postenable = &stm32_dfsdm_postenable,
>> + .predisable = &stm32_dfsdm_predisable,
>> +};
>> +
>> +/**
>> + * stm32_dfsdm_get_buff_cb() - register a callback that will be called when
>> + * DMA transfer period is achieved.
>> + *
>> + * @iio_dev: Handle to IIO device.
>> + * @cb: Pointer to callback function:
>> + * - data: pointer to data buffer
>> + * - size: size in byte of the data buffer
>> + * - private: pointer to consumer private structure.
>> + * @private: Pointer to consumer private structure.
>> + */
>> +int stm32_dfsdm_get_buff_cb(struct iio_dev *iio_dev,
>> + int (*cb)(const void *data, size_t size,
>> + void *private),
>> + void *private)
>> +{
>> + struct stm32_dfsdm_adc *adc;
>> +
>> + if (!iio_dev)
>> + return -EINVAL;
>> + adc = iio_priv(iio_dev);
>> +
>> + adc->cb = cb;
>> + adc->cb_priv = private;
>> +
>> + return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(stm32_dfsdm_get_buff_cb);
>> +
>> +/**
>> + * stm32_dfsdm_release_buff_cb - unregister buffer callback
>> + *
>> + * @iio_dev: Handle to IIO device.
>> + */
>> +int stm32_dfsdm_release_buff_cb(struct iio_dev *iio_dev)
>> +{
>> + struct stm32_dfsdm_adc *adc;
>> +
>> + if (!iio_dev)
>> + return -EINVAL;
>> + adc = iio_priv(iio_dev);
>> +
>> + adc->cb = NULL;
>> + adc->cb_priv = NULL;
>> +
>> + return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(stm32_dfsdm_release_buff_cb);
>> +
>> static int stm32_dfsdm_single_conv(struct iio_dev *indio_dev,
>> const struct iio_chan_spec *chan, int *res)
>> {
>> @@ -453,15 +767,41 @@ static int stm32_dfsdm_write_raw(struct iio_dev *indio_dev,
>> {
>> struct stm32_dfsdm_adc *adc = iio_priv(indio_dev);
>> struct stm32_dfsdm_filter *fl = &adc->dfsdm->fl_list[adc->fl_id];
>> + struct stm32_dfsdm_channel *ch = &adc->dfsdm->ch_list[adc->ch_id];
>> + unsigned int spi_freq = adc->spi_freq;
>> int ret = -EINVAL;
>>
>> - if (mask == IIO_CHAN_INFO_OVERSAMPLING_RATIO) {
>> + switch (mask) {
>> + case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
>> ret = stm32_dfsdm_set_osrs(fl, 0, val);
>> if (!ret)
>> adc->oversamp = val;
>> +
>> + return ret;
>> +
>> + case IIO_CHAN_INFO_SAMP_FREQ:
>> + if (!val)
>> + return -EINVAL;
>> + if (ch->src != DFSDM_CHANNEL_SPI_CLOCK_EXTERNAL)
>> + spi_freq = adc->dfsdm->spi_master_freq;
>> +
>> + if (spi_freq % val)
>> + dev_warn(&indio_dev->dev,
>> + "Sampling rate not accurate (%d)\n",
>> + spi_freq / (spi_freq / val));
>> +
>> + ret = stm32_dfsdm_set_osrs(fl, 0, (spi_freq / val));
>> + if (ret < 0) {
>> + dev_err(&indio_dev->dev,
>> + "Not able to find parameter that match!\n");
>> + return ret;
>> + }
>> + adc->sample_freq = val;
>> +
>> + return 0;
>> }
>>
>> - return ret;
>> + return -EINVAL;
>> }
>>
>> static int stm32_dfsdm_read_raw(struct iio_dev *indio_dev,
>> @@ -494,11 +834,22 @@ static int stm32_dfsdm_read_raw(struct iio_dev *indio_dev,
>> *val = adc->oversamp;
>>
>> return IIO_VAL_INT;
>> +
>> + case IIO_CHAN_INFO_SAMP_FREQ:
>> + *val = adc->sample_freq;
>> +
>> + return IIO_VAL_INT;
>> }
>>
>> return -EINVAL;
>> }
>>
>> +static const struct iio_info stm32_dfsdm_info_audio = {
>> + .hwfifo_set_watermark = stm32_dfsdm_set_watermark,
>> + .read_raw = stm32_dfsdm_read_raw,
>> + .write_raw = stm32_dfsdm_write_raw,
>> +};
>> +
>> static const struct iio_info stm32_dfsdm_info_adc = {
>> .read_raw = stm32_dfsdm_read_raw,
>> .write_raw = stm32_dfsdm_write_raw,
>> @@ -531,6 +882,60 @@ static irqreturn_t stm32_dfsdm_irq(int irq, void *arg)
>> return IRQ_HANDLED;
>> }
>>
>> +/*
>> + * Define external info for SPI Frequency and audio sampling rate that can be
>> + * configured by ASoC driver through consumer.h API
>> + */
>> +static const struct iio_chan_spec_ext_info dfsdm_adc_audio_ext_info[] = {
>> + /* spi_clk_freq : clock freq on SPI/manchester bus used by channel */
>> + {
>> + .name = "spi_clk_freq",
>> + .shared = IIO_SHARED_BY_TYPE,
>> + .read = dfsdm_adc_audio_get_spiclk,
>> + .write = dfsdm_adc_audio_set_spiclk,
>> + },
>> + {},
>> +};
>> +
>> +static int stm32_dfsdm_dma_request(struct iio_dev *indio_dev)
>> +{
>> + struct stm32_dfsdm_adc *adc = iio_priv(indio_dev);
>> + struct dma_slave_config config;
>> + int ret;
>> +
>> + adc->dma_chan = dma_request_slave_channel(&indio_dev->dev, "rx");
>> + if (!adc->dma_chan)
>> + return -EINVAL;
>> +
>> + adc->rx_buf = dma_alloc_coherent(adc->dma_chan->device->dev,
>> + DFSDM_DMA_BUFFER_SIZE,
>> + &adc->dma_buf, GFP_KERNEL);
>> + if (!adc->rx_buf) {
>> + ret = -ENOMEM;
>> + goto err_release;
>> + }
>> +
>> + /* Configure DMA channel to read data register */
>> + memset(&config, 0, sizeof(config));
>> + config.src_addr = (dma_addr_t)adc->dfsdm->phys_base;
>> + config.src_addr += DFSDM_RDATAR(adc->fl_id);
>> + config.src_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
>> +
>> + ret = dmaengine_slave_config(adc->dma_chan, &config);
>> + if (ret)
>> + goto err_free;
>> +
>> + return 0;
>> +
>> +err_free:
>> + dma_free_coherent(adc->dma_chan->device->dev, DFSDM_DMA_BUFFER_SIZE,
>> + adc->rx_buf, adc->dma_buf);
>> +err_release:
>> + dma_release_channel(adc->dma_chan);
>> +
>> + return ret;
>> +}
>> +
>> static int stm32_dfsdm_adc_chan_init_one(struct iio_dev *indio_dev,
>> struct iio_chan_spec *ch)
>> {
>> @@ -551,7 +956,12 @@ static int stm32_dfsdm_adc_chan_init_one(struct iio_dev *indio_dev,
>> ch->info_mask_separate = BIT(IIO_CHAN_INFO_RAW);
>> ch->info_mask_shared_by_all = BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO);
>>
>> - ch->scan_type.sign = 'u';
>> + if (adc->dev_data->type == DFSDM_AUDIO) {
>> + ch->scan_type.sign = 's';
>> + ch->ext_info = dfsdm_adc_audio_ext_info;
>> + } else {
>> + ch->scan_type.sign = 'u';
>> + }
>> ch->scan_type.realbits = 24;
>> ch->scan_type.storagebits = 32;
>> adc->ch_id = ch->channel;
>> @@ -560,6 +970,64 @@ static int stm32_dfsdm_adc_chan_init_one(struct iio_dev *indio_dev,
>> &adc->dfsdm->ch_list[ch->channel]);
>> }
>>
>> +static int stm32_dfsdm_audio_init(struct iio_dev *indio_dev)
>> +{
>> + struct iio_chan_spec *ch;
>> + struct stm32_dfsdm_adc *adc = iio_priv(indio_dev);
>> + struct stm32_dfsdm_channel *d_ch;
>> + int ret;
>> +
>> + ret = stm32_dfsdm_dma_request(indio_dev);
>> + if (ret) {
>> + dev_err(&indio_dev->dev, "DMA request failed\n");
>> + return ret;
>> + }
>> +
>> + indio_dev->modes |= INDIO_BUFFER_SOFTWARE;
>> +
>> + ret = iio_triggered_buffer_setup(indio_dev,
>> + &iio_pollfunc_store_time,
>> + NULL,
>> + &stm32_dfsdm_buffer_setup_ops);
>> + if (ret) {
>> + dev_err(&indio_dev->dev, "Buffer setup failed\n");
>> + goto err_dma_disable;
>> + }
>> +
>> + ch = devm_kzalloc(&indio_dev->dev, sizeof(*ch), GFP_KERNEL);
>> + if (!ch)
>> + return -ENOMEM;
>> +
>> + ch->scan_index = 0;
>> + ret = stm32_dfsdm_adc_chan_init_one(indio_dev, ch);
>> + if (ret < 0) {
>> + dev_err(&indio_dev->dev, "channels init failed\n");
>> + goto err_buffer_cleanup;
>> + }
>> + ch->info_mask_separate = BIT(IIO_CHAN_INFO_SAMP_FREQ);
>> +
>> + d_ch = &adc->dfsdm->ch_list[adc->ch_id];
>> + if (d_ch->src != DFSDM_CHANNEL_SPI_CLOCK_EXTERNAL)
>> + adc->spi_freq = adc->dfsdm->spi_master_freq;
>> +
>> + indio_dev->num_channels = 1;
>> + indio_dev->channels = ch;
>> +
>> + return 0;
>> +
>> +err_buffer_cleanup:
>> + iio_triggered_buffer_cleanup(indio_dev);
>> +
>> +err_dma_disable:
>> + if (adc->dma_chan) {
>> + dma_free_coherent(adc->dma_chan->device->dev,
>> + DFSDM_DMA_BUFFER_SIZE,
>> + adc->rx_buf, adc->dma_buf);
>> + dma_release_channel(adc->dma_chan);
>> + }
>> + return ret;
>> +}
>> +
>> static int stm32_dfsdm_adc_init(struct iio_dev *indio_dev)
>> {
>> struct iio_chan_spec *ch;
>> @@ -612,11 +1080,20 @@ static const struct stm32_dfsdm_dev_data stm32h7_dfsdm_adc_data = {
>> .init = stm32_dfsdm_adc_init,
>> };
>>
>> +static const struct stm32_dfsdm_dev_data stm32h7_dfsdm_audio_data = {
>> + .type = DFSDM_AUDIO,
>> + .init = stm32_dfsdm_audio_init,
>> +};
>> +
>> static const struct of_device_id stm32_dfsdm_adc_match[] = {
>> {
>> .compatible = "st,stm32-dfsdm-adc",
>> .data = &stm32h7_dfsdm_adc_data,
>> },
>> + {
>> + .compatible = "st,stm32-dfsdm-dmic",
>> + .data = &stm32h7_dfsdm_audio_data,
>> + },
>> {}
>> };
>>
>> @@ -667,8 +1144,13 @@ static int stm32_dfsdm_adc_probe(struct platform_device *pdev)
>> name = devm_kzalloc(dev, sizeof("dfsdm-adc0"), GFP_KERNEL);
>> if (!name)
>> return -ENOMEM;
>> - iio->info = &stm32_dfsdm_info_adc;
>> - snprintf(name, sizeof("dfsdm-adc0"), "dfsdm-adc%d", adc->fl_id);
>> + if (dev_data->type == DFSDM_AUDIO) {
>> + iio->info = &stm32_dfsdm_info_audio;
>> + snprintf(name, sizeof("dfsdm-pdm0"), "dfsdm-pdm%d", adc->fl_id);
>> + } else {
>> + iio->info = &stm32_dfsdm_info_adc;
>> + snprintf(name, sizeof("dfsdm-adc0"), "dfsdm-adc%d", adc->fl_id);
>> + }
>> iio->name = name;
>>
>> /*
>> @@ -700,7 +1182,10 @@ static int stm32_dfsdm_adc_probe(struct platform_device *pdev)
>> if (ret < 0)
>> return ret;
>>
>> - return iio_device_register(iio);
>> + iio_device_register(iio);
>> + if (dev_data->type == DFSDM_AUDIO)
>> + return devm_of_platform_populate(&pdev->dev);
>> + return 0;
>> }
>>
>> static int stm32_dfsdm_adc_remove(struct platform_device *pdev)
>> @@ -709,7 +1194,14 @@ static int stm32_dfsdm_adc_remove(struct platform_device *pdev)
>> struct iio_dev *indio_dev = iio_priv_to_dev(adc);
>>
>> iio_device_unregister(indio_dev);
>> -
>> + if (indio_dev->pollfunc)
>> + iio_triggered_buffer_cleanup(indio_dev);
>> + if (adc->dma_chan) {
>> + dma_free_coherent(adc->dma_chan->device->dev,
>> + DFSDM_DMA_BUFFER_SIZE,
>> + adc->rx_buf, adc->dma_buf);
>> + dma_release_channel(adc->dma_chan);
>> + }
>> return 0;
>> }
>>
>> diff --git a/include/linux/iio/adc/stm32-dfsdm-adc.h b/include/linux/iio/adc/stm32-dfsdm-adc.h
>> new file mode 100644
>> index 0000000..e7dc7a5
>> --- /dev/null
>> +++ b/include/linux/iio/adc/stm32-dfsdm-adc.h
>> @@ -0,0 +1,18 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +/*
>> + * This file discribe the STM32 DFSDM IIO driver API for audio part
>> + *
>> + * Copyright (C) 2017, STMicroelectronics - All Rights Reserved
>> + * Author(s): Arnaud Pouliquen <arnaud.pouliquen@st.com>.
>> + */
>> +
>> +#ifndef STM32_DFSDM_ADC_H
>> +#define STM32_DFSDM_ADC_H
>> +
>> +int stm32_dfsdm_get_buff_cb(struct iio_dev *iio_dev,
>> + int (*cb)(const void *data, size_t size,
>> + void *private),
>> + void *private);
>> +int stm32_dfsdm_release_buff_cb(struct iio_dev *iio_dev);
>> +
>> +#endif
>
^ permalink raw reply
* Re: [PATCH 1/1] dt-bindings: arm: document supported STM32 SoC family
From: Ludovic BARRE @ 2017-12-13 8:40 UTC (permalink / raw)
To: Rob Herring
Cc: Mark Rutland, Maxime Coquelin, Alexandre Torgue,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Gwenael Treuveur
In-Reply-To: <20171212231744.u4wtadrpaq3wp4sq@rob-hp-laptop>
hi Rob
Today there was no bindings documentation for STM32 SoC
already upstreamed. This patch adds initial list of STM32
existing in kernel.
The next serie adds just new soc and one compatible on STM32 list.
So, I think you could apply this patch.
BR
Ludo
On 12/13/2017 12:17 AM, Rob Herring wrote:
> On Tue, Dec 12, 2017 at 03:03:48PM -0600, Rob Herring wrote:
>> On Fri, Dec 08, 2017 at 02:56:34PM +0100, Ludovic Barre wrote:
>>> From: Ludovic Barre <ludovic.barre-qxv4g6HH51o@public.gmane.org>
>>>
>>> This adds a list of supported STM32 SoC bindings.
>>>
>>> Signed-off-by: Gwenael Treuveur <gwenael.treuveur-qxv4g6HH51o@public.gmane.org>
>>> Signed-off-by: Ludovic Barre <ludovic.barre-qxv4g6HH51o@public.gmane.org>
>>> ---
>>> Documentation/devicetree/bindings/arm/stm32.txt | 9 +++++++++
>>> 1 file changed, 9 insertions(+)
>>> create mode 100644 Documentation/devicetree/bindings/arm/stm32.txt
>>
>> Applied, thanks.
>
> Now dropped as this will conflict with your other series. Send this with
> the other series or indicate who should apply.
>
> Reviewed-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>
> Rob
>
--
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] dt-bindings: timer: renesas, cmt: Document r8a774[35] CMT support
From: Simon Horman @ 2017-12-13 8:40 UTC (permalink / raw)
To: Fabrizio Castro
Cc: Rob Herring, Mark Rutland, linux-renesas-soc, Daniel Lezcano,
Thomas Gleixner, Geert Uytterhoeven, Chris Paterson, Biju Das,
devicetree
In-Reply-To: <1513104579-6333-2-git-send-email-fabrizio.castro@bp.renesas.com>
On Tue, Dec 12, 2017 at 06:49:37PM +0000, Fabrizio Castro wrote:
> Document SoC specific compatible strings for r8a7743 and r8a7745.
> No driver change is needed as the fallback strings will activate
> the right code.
>
> Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> Reviewed-by: Biju Das <biju.das@bp.renesas.com>
> ---
> Documentation/devicetree/bindings/timer/renesas,cmt.txt | 12 +++++++++---
> 1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/timer/renesas,cmt.txt b/Documentation/devicetree/bindings/timer/renesas,cmt.txt
> index d740989..1e4fe98 100644
> --- a/Documentation/devicetree/bindings/timer/renesas,cmt.txt
> +++ b/Documentation/devicetree/bindings/timer/renesas,cmt.txt
> @@ -22,6 +22,10 @@ Required Properties:
>
> - "renesas,r8a73a4-cmt0" for the 32-bit CMT0 device included in r8a73a4.
> - "renesas,r8a73a4-cmt1" for the 48-bit CMT1 device included in r8a73a4.
> + - "renesas,r8a7743-cmt0" for the 32-bit CMT0 device included in r8a7743.
> + - "renesas,r8a7743-cmt1" for the 48-bit CMT1 device included in r8a7743.
> + - "renesas,r8a7745-cmt0" for the 32-bit CMT0 device included in r8a7745.
> + - "renesas,r8a7745-cmt1" for the 48-bit CMT1 device included in r8a7745.
> - "renesas,r8a7790-cmt0" for the 32-bit CMT0 device included in r8a7790.
> - "renesas,r8a7790-cmt1" for the 48-bit CMT1 device included in r8a7790.
> - "renesas,r8a7791-cmt0" for the 32-bit CMT0 device included in r8a7791.
> @@ -31,9 +35,11 @@ Required Properties:
> - "renesas,r8a7794-cmt0" for the 32-bit CMT0 device included in r8a7794.
> - "renesas,r8a7794-cmt1" for the 48-bit CMT1 device included in r8a7794.
>
> - - "renesas,rcar-gen2-cmt0" for 32-bit CMT0 devices included in R-Car Gen2.
> - - "renesas,rcar-gen2-cmt1" for 48-bit CMT1 devices included in R-Car Gen2.
> - These are fallbacks for r8a73a4 and all the R-Car Gen2
> + - "renesas,rcar-gen2-cmt0" for 32-bit CMT0 devices included in R-Car Gen2 or
nit: or -> and ?
> + RZ/G1.
> + - "renesas,rcar-gen2-cmt1" for 48-bit CMT1 devices included in R-Car Gen2 or
> + RZ/G1.
> + These are fallbacks for r8a73a4, all the R-Car Gen2 and RZ/G1
> entries listed above.
nit: all the R-Car -> R-Car
> - reg: base address and length of the registers block for the timer module.
^ permalink raw reply
* Lithium battery protection was Re: [PATCH v4.14] Add support for bq27521 battery monitor
From: Pavel Machek @ 2017-12-13 8:39 UTC (permalink / raw)
To: Sebastian Reichel
Cc: Andrew F. Davis, pali.rohar-Re5JQEeQqe8AvxtiuMwx3w, kernel list,
linux-arm-kernel, linux-omap-u79uwXL29TY76Z2rM5mHXA,
tony-4v6yS6AI5VpBDgjK7y7TUQ, khilman-DgEjT+Ai2ygdnm+yROfE0A,
aaro.koskinen-X3B1VOXEql0,
ivo.g.dimitrov.75-Re5JQEeQqe8AvxtiuMwx3w,
patrikbachan-Re5JQEeQqe8AvxtiuMwx3w, serge-A9i7LUbDfNHQT0dZR+AlfA,
abcloriens-Re5JQEeQqe8AvxtiuMwx3w, clayton-fehKsxFhGzZIf6P1QZMOBw,
martijn-28JJ9oSIdodmR6Xm/wNWPw,
sakari.ailus-VuQAYsv1563Yd54FQh9/CA,
kernel-RYWXG+zxWwBdeoIcmNTgJF6hYfS7NtTn,
devicetree-u79uwXL29TY76Z2rM5mHXA, robh+dt-DgEjT+Ai2ygdnm+yROfE0A
In-Reply-To: <20171208170610.gbmlsgm7xzh2zpyn@earth>
[-- Attachment #1: Type: text/plain, Size: 1355 bytes --]
Hi!
> > >> This adds basic support for BQ27521 battery monitor, used in Nokia N9
> > >> and N950. In particular, battery voltage is important to be able to
> > >> tell when the battery is almost empty. Emptying battery on N950 is
> > >> pretty painful, as flasher needs to be used to recover phone in such
> > >> case.
> > >
> > > Sebastian, ping? This one should not be too controversial.
> >
> > Acked-by: Andrew F. Davis <afd-l0cyMroinI0@public.gmane.org>
>
> Thanks, queued, I dropped the spurious change in twl.h and added the
> dt-binding from the previous patch version, that was lost somehow.
Thanks!
> > > If you could also apply the "shut down when battery is low", that
> > > would be nice.
>
> I only have one with values specific to N900 hardcoded in the
> driver. That one can't be applied for obvious reasons.
Well... the values are not really N900-specific. They should work well
on anything with lithium battery, because they basically depend only
on battery chemistry.
I guess I can simplify patch to work with battery voltage only --
that's what hardware battery protection does -- and make shutdown
voltage configurable ...?
Best regards,
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply
* Re: [PATCH 0/2] Bluetooth: Add device tree compatible for TI CC2560
From: Marcel Holtmann @ 2017-12-13 8:39 UTC (permalink / raw)
To: David Lechner
Cc: devicetree, open list:BLUETOOTH DRIVERS, Rob Herring,
Mark Rutland, Johan Hedberg, netdev, linux-kernel
In-Reply-To: <1513124971-23717-1-git-send-email-david@lechnology.com>
Hi David,
> This series updates the bindings TI WiLink 7/8 Bluetooth to add TI CC256x
> chips as well. A compatible string is also added to the hci_ll driver for
> TI CC2560.
>
> David Lechner (2):
> dt-bindings: net: add TI CC2560 Bluetooth chip
> Bluetooth: hci_ll: add "ti,cc2560" compatible string
>
> .../bindings/net/{ti,wilink-st.txt => ti-bluetooth.txt} | 13 +++++++++++--
> drivers/bluetooth/hci_ll.c | 1 +
> 2 files changed, 12 insertions(+), 2 deletions(-)
> rename Documentation/devicetree/bindings/net/{ti,wilink-st.txt => ti-bluetooth.txt} (78%)
both patches have been applied to bluetooth-next tree.
Regards
Marcel
^ permalink raw reply
* Re: [PATCH v2 3/4] thermal: armada: add support for CP110
From: Baruch Siach @ 2017-12-13 8:38 UTC (permalink / raw)
To: Gregory CLEMENT
Cc: Ezequiel Garcia, Miquel RAYNAL, Zhang Rui, Eduardo Valentin,
devicetree-u79uwXL29TY76Z2rM5mHXA, Jason Cooper, Andrew Lunn,
Sebastian Hesselbarth, Russell King,
linux-pm-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <87y3m9qjt2.fsf-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
Hi Gregory,
On Mon, Dec 11, 2017 at 06:02:49PM +0100, Gregory CLEMENT wrote:
> On lun., déc. 11 2017, Baruch Siach <baruch-NswTu9S1W3P6gbPvEgmw2w@public.gmane.org> wrote:
> > On Mon, Dec 11, 2017 at 04:09:32PM +0100, Miquel RAYNAL wrote:
> >> On Sun, 3 Dec 2017 13:11:23 +0200
> >> Baruch Siach <baruch-NswTu9S1W3P6gbPvEgmw2w@public.gmane.org> wrote:
> >>
> >> > The CP110 component is integrated in the Armada 8k and 7k lines of
> >> > processors.
> >> >
> >> > This patch also adds an option of offset to the MSB of the control
> >> > register. The existing DT binding for Armada 38x refers to a single
> >> > 32 bit control register. It turns out that this is actually only the
> >> > MSB of the control area. Changing the binding to fix that would break
> >> > existing DT files, so the Armada 38x binding is left as is.
> >> >
> >> > The new CP110 binding increases the size of the control area to 64
> >> > bits, thus moving the MSB to offset 4.
> >> >
> >> > Signed-off-by: Baruch Siach <baruch-NswTu9S1W3P6gbPvEgmw2w@public.gmane.org>
> >> > ---
> >> > v2: No change
> >> > ---
> >> > drivers/thermal/armada_thermal.c | 24 ++++++++++++++++++++++--
> >> > 1 file changed, 22 insertions(+), 2 deletions(-)
> >> >
> >> > diff --git a/drivers/thermal/armada_thermal.c
> >> > b/drivers/thermal/armada_thermal.c index 0eb82097571f..59b75f63945d
> >> > 100644 --- a/drivers/thermal/armada_thermal.c
> >> > +++ b/drivers/thermal/armada_thermal.c
> >> > @@ -73,6 +73,7 @@ struct armada_thermal_data {
> >> > unsigned int temp_shift;
> >> > unsigned int temp_mask;
> >> > unsigned int is_valid_shift;
> >> > + unsigned int control_msb_offset;
> >> > };
> >> >
> >> > static void armadaxp_init_sensor(struct platform_device *pdev,
> >> > @@ -142,12 +143,14 @@ static void armada375_init_sensor(struct
> >> > platform_device *pdev, static void armada380_init_sensor(struct
> >> > platform_device *pdev, struct armada_thermal_priv *priv)
> >> > {
> >> > - unsigned long reg = readl_relaxed(priv->control);
> >> > + void __iomem *control_msb =
> >> > + priv->control + priv->data->control_msb_offset;
> >> > + unsigned long reg = readl_relaxed(control_msb);
> >> >
> >> > /* Reset hardware once */
> >> > if (!(reg & A380_HW_RESET)) {
> >> > reg |= A380_HW_RESET;
> >> > - writel(reg, priv->control);
> >> > + writel(reg, control_msb);
> >> > mdelay(10);
> >> > }
> >> > }
> >> > @@ -266,6 +269,19 @@ static const struct armada_thermal_data
> >> > armada_ap806_data = { .signed_sample = true,
> >> > };
> >> >
> >> > +static const struct armada_thermal_data armada_cp110_data = {
> >> > + .is_valid = armada_is_valid,
> >> > + .init_sensor = armada380_init_sensor,
> >>
> >> I see the initialization for CP110 thermal IP is close to
> >> Armada-380's, but, as you point it in the commit log it is still
> >> different.
> >>
> >> I don't know what is the best way to handle this but until now each
> >> new compatible had his own ->init_sensor function, shouldn't we do
> >> the same here as changes are requested? This would naturally avoid the
> >> situation with Armada-380 bindings.
> >
> > I'm not sure I understand your suggestion.
> >
> > There is no difference between the CP110 and the Armada 38x, as far as I can
> > see. The only quirk is that the existing Armada 38x DT binding is wrong I that
> > the 'reg' property references the control MSB, while leaving the LSB
> > out. We
>
> Well I would not say it was wrong but more incomplete :)
>
> > can't change the Armada 38x binding without breaking existing DTs. The
> > 'control_msb_offset' field that this patch adds allows correct binding for
> > CP110, while keeping compatibility with the existing Armada 38x
> > binding.
>
> I am not against adding a new compatible string for CP110 but ot be
> honest the new binding for CP110 does not bring anything as you don't
> use at all the LSB register.
We don't use the LSB yet in mainline driver. But the vendor kernel uses it to
"change temperature band gap circuit curve" (quoting vendor kernel commit
4ff2d8a7d3 log). Chances are that we want to do this as well. But said commit
changed the DT binding in an incompatible way. We can't do that, and we both
agree on that.
> Actually, if on Armada 375 we initially mapped the LSB register it was
> to support an very early release of the SoC (stepping Z) and only for
> resetting its value. So I guess you started to write the AP860 part
> based on the Armada 375 and then found that we could map a more complete
> range of the registers.
>
> > How would a separate init_sensor routine improve things?
>
> So yes please do it, thanks to this you won't have to add the
> control_msb_offset member and can use a clean function. Moreover if in
> the future we see some usefulness for this LSB register then we could use
> the new compatible for the Armada 38x.
There are two separate issues here:
1. DT binding
2. init_sensor callback implementation
We both agree on #1. The A38x and CP110 need separate compatible strings. In
case we want to access the LSB control register on Armada 38x, we will need
yet another compatible string (marvell,armada380-v2-thermal maybe?).
As for #2, I'm all for sharing as much code as possible. I find the vendor
kernel approach of duplicating the init routines[1] unhelpful as it violates
the DRY principle. The differences between armada380_init_sensor() and
cp110_init_sensor() are minor. In my opinion, these differences should be
expressed explicitly in the armada_thermal_data, in a similar way to my
suggested control_msb_offset field. The vendor code hides these differences in
slight variations of duplicated code.
What is the advantage of a separate init routine?
baruch
[1] https://github.com/MarvellEmbeddedProcessors/linux-marvell/blob/linux-4.4.52-armada-17.10/drivers/thermal/armada_thermal.c
--
http://baruch.siach.name/blog/ ~. .~ Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
- baruch-NswTu9S1W3P6gbPvEgmw2w@public.gmane.org - tel: +972.2.679.5364, http://www.tkos.co.il -
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 2/3] ARM: dts: r8a7743: Add CMT SoC specific support
From: Simon Horman @ 2017-12-13 8:38 UTC (permalink / raw)
To: Fabrizio Castro
Cc: Rob Herring, Mark Rutland, Russell King, Magnus Damm,
Geert Uytterhoeven, Chris Paterson, Biju Das, linux-renesas-soc,
devicetree, linux-arm-kernel
In-Reply-To: <1513104579-6333-3-git-send-email-fabrizio.castro@bp.renesas.com>
On Tue, Dec 12, 2017 at 06:49:38PM +0000, Fabrizio Castro wrote:
> Add CMT[01] support to SoC DT.
>
> Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> Reviewed-by: Biju Das <biju.das@bp.renesas.com>
> ---
> arch/arm/boot/dts/r8a7743.dtsi | 30 ++++++++++++++++++++++++++++++
> 1 file changed, 30 insertions(+)
I was expecting the cmt nodes to be "disabled" in the SoC file
and then enabled selectively in board files. Am I missing something?
Otherwise this patch looks good to me.
> diff --git a/arch/arm/boot/dts/r8a7743.dtsi b/arch/arm/boot/dts/r8a7743.dtsi
> index 59860c8..0e2834a 100644
> --- a/arch/arm/boot/dts/r8a7743.dtsi
> +++ b/arch/arm/boot/dts/r8a7743.dtsi
> @@ -262,6 +262,36 @@
> IRQ_TYPE_LEVEL_LOW)>;
> };
>
> + cmt0: timer@ffca0000 {
> + compatible = "renesas,r8a7743-cmt0",
> + "renesas,rcar-gen2-cmt0";
> + reg = <0 0xffca0000 0 0x1004>;
> + interrupts = <GIC_SPI 142 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 143 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&cpg CPG_MOD 124>;
> + clock-names = "fck";
> + power-domains = <&sysc R8A7743_PD_ALWAYS_ON>;
> + resets = <&cpg 124>;
> + };
> +
> + cmt1: timer@e6130000 {
> + compatible = "renesas,r8a7743-cmt1",
> + "renesas,rcar-gen2-cmt1";
> + reg = <0 0xe6130000 0 0x1004>;
> + interrupts = <GIC_SPI 120 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 121 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 122 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 123 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 124 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 125 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 126 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 127 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&cpg CPG_MOD 329>;
> + clock-names = "fck";
> + power-domains = <&sysc R8A7743_PD_ALWAYS_ON>;
> + resets = <&cpg 329>;
> + };
> +
> cpg: clock-controller@e6150000 {
> compatible = "renesas,r8a7743-cpg-mssr";
> reg = <0 0xe6150000 0 0x1000>;
> --
> 2.7.4
>
^ permalink raw reply
* Re: [PATCH v3 09/33] nds32: Cache and TLB routines
From: Greentime Hu @ 2017-12-13 8:30 UTC (permalink / raw)
To: Guo Ren
Cc: Greentime, Linux Kernel Mailing List, Arnd Bergmann, linux-arch,
Thomas Gleixner, Jason Cooper, Marc Zyngier, Rob Herring, netdev,
Vincent Chen, DTML, Al Viro, David Howells, Will Deacon,
Daniel Lezcano, linux-serial, Geert Uytterhoeven, Linus Walleij,
Mark Rutland, Greg KH
In-Reply-To: <20171213081949.GA18840@gary-OptiPlex-3050>
2017-12-13 16:19 GMT+08:00 Guo Ren <ren_guo@c-sky.com>:
> On Wed, Dec 13, 2017 at 01:45:02PM +0800, Greentime Hu wrote:
>
>> I think it should be fine if an interruption between mtsr_dsb and
>> tlbop_rwr because this is a optimization by sw.
>
> Fine? When there is an unexpected vaddr in SR_TLB_VPN, tlbop_rwr(*pte) will
> break that vaddr's pfn in the CPU tlb-buffer entry. When linux access the
> vaddr, it will get wrong data unless the entry has been replaced out.
Hi, Guo Ren:
Thanks. I get your point.
It is needed to be protected.
I will fix it in the next version patch.
if (vma->vm_mm == current->active_mm) {
local_irq_save(flags);
__nds32__mtsr_dsb(addr, NDS32_SR_TLB_VPN);
__nds32__tlbop_rwr(*pte);
__nds32__isb();
local_irq_restore(flags);
}
^ permalink raw reply
* Re: [PATCH v3 09/33] nds32: Cache and TLB routines
From: Guo Ren @ 2017-12-13 8:19 UTC (permalink / raw)
To: Greentime Hu
Cc: Greentime, Linux Kernel Mailing List, Arnd Bergmann, linux-arch,
Thomas Gleixner, Jason Cooper, Marc Zyngier, Rob Herring, netdev,
Vincent Chen, DTML, Al Viro, David Howells, Will Deacon,
Daniel Lezcano, linux-serial-u79uwXL29TY76Z2rM5mHXA,
Geert Uytterhoeven, Linus Walleij, Mark Rutland, Greg KH
In-Reply-To: <CAEbi=3dtcnUNbd4SoueUnoYvRWot9fA1n62t0b4PWx1UYs2jZA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Wed, Dec 13, 2017 at 01:45:02PM +0800, Greentime Hu wrote:
> I think it should be fine if an interruption between mtsr_dsb and
> tlbop_rwr because this is a optimization by sw.
Fine? When there is an unexpected vaddr in SR_TLB_VPN, tlbop_rwr(*pte) will
break that vaddr's pfn in the CPU tlb-buffer entry. When linux access the
vaddr, it will get wrong data unless the entry has been replaced out.
--
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] dt-bindings: pinctrl: stm32: fix copyright and adopt SPDX identifier
From: Linus Walleij @ 2017-12-13 8:16 UTC (permalink / raw)
To: Alexandre Torgue
Cc: Maxime Coquelin, Rob Herring, Mark Rutland, Linux ARM,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <1512748391-22004-1-git-send-email-alexandre.torgue-qxv4g6HH51o@public.gmane.org>
On Fri, Dec 8, 2017 at 4:53 PM, Alexandre Torgue
<alexandre.torgue-qxv4g6HH51o@public.gmane.org> wrote:
> Add missing copyright and add SPDX identifier.
>
> Signed-off-by: Alexandre Torgue <alexandre.torgue-qxv4g6HH51o@public.gmane.org>
Patch applied.
Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [RFC PATCH 1/2] dt: bindings: as3645a: Update dt node example with standard
From: Laurent Pinchart @ 2017-12-13 8:09 UTC (permalink / raw)
To: Dan Murphy
Cc: robh+dt, mark.rutland, rpurdie, jacek.anaszewski, pavel,
sakari.ailus, devicetree, linux-kernel, linux-leds
In-Reply-To: <20171212215024.30116-1-dmurphy@ti.com>
Hi Dan,
Thank you for the patch.
On Tuesday, 12 December 2017 23:50:23 EET Dan Murphy wrote:
> Update the DT binding to remove the device name from
> the DT parent node as well as removing the device
> name from the label. The LED label will be generated
> based off the id name stored in the local driver so
> the LED function can be indicated in the label DT
> entry.
>
> Also removed the indentation on the example.
This makes the patch a bit harder to review and seems to be a matter of style.
> Signed-off-by: Dan Murphy <dmurphy@ti.com>
> ---
> .../devicetree/bindings/leds/ams,as3645a.txt | 36 ++++++++++---------
> 1 file changed, 18 insertions(+), 18 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/leds/ams,as3645a.txt
> b/Documentation/devicetree/bindings/leds/ams,as3645a.txt index
> fc7f5f9f234c..122aa7165cf3 100644
> --- a/Documentation/devicetree/bindings/leds/ams,as3645a.txt
> +++ b/Documentation/devicetree/bindings/leds/ams,as3645a.txt
> @@ -58,22 +58,22 @@ label : The label of the indicator LED.
I believe you should expand the documentation of the label property to detail
how it should be formed. It's nice to update the example, but the bindings
should be understandable without it.
> Example
> =======
>
> - as3645a@30 {
> - compatible = "ams,as3645a";
> - #address-cells = <1>;
> - #size-cells = <0>;
> - reg = <0x30>;
> - flash@0 {
> - reg = <0x0>;
> - flash-timeout-us = <150000>;
> - flash-max-microamp = <320000>;
> - led-max-microamp = <60000>;
> - ams,input-max-microamp = <1750000>;
> - label = "as3645a:flash";
> - };
> - indicator@1 {
> - reg = <0x1>;
> - led-max-microamp = <10000>;
> - label = "as3645a:indicator";
> - };
> +led-controller@30 {
This change looks fine to me.
> + compatible = "ams,as3645a";
> + #address-cells = <1>;
> + #size-cells = <0>;
> + reg = <0x30>;
> + led@0 {
What's the rationale for changing the node name here ? It should be explained
in the commit message, and in the DT bindings documentation.
> + reg = <0x0>;
> + flash-timeout-us = <150000>;
> + flash-max-microamp = <320000>;
> + led-max-microamp = <60000>;
> + ams,input-max-microamp = <1750000>;
> + label = "flash";
> };
> + led@1 {
> + reg = <0x1>;
> + led-max-microamp = <10000>;
> + label = "indicator";
> + };
> +};
--
Regards,
Laurent Pinchart
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox