Devicetree
 help / color / mirror / Atom feed
* Re: [V6, 1/2] media: dt-bindings: media: i2c: Document DW9768 bindings
From: Tomasz Figa @ 2020-05-18 14:12 UTC (permalink / raw)
  To: Dongchun Zhu
  Cc: Linus Walleij, Bartosz Golaszewski, Mauro Carvalho Chehab,
	Andy Shevchenko, Rob Herring, Mark Rutland, Sakari Ailus,
	Nicolas Boichat, Matthias Brugger, Cao Bing Bu, srv_heupstream,
	moderated list:ARM/Mediatek SoC support,
	list@263.net:IOMMU DRIVERS <iommu@lists.linux-foundation.org>, Joerg Roedel <joro@8bytes.org>,,
	Sj Huang, Linux Media Mailing List, linux-devicetree, Louis Kuo,
	Shengnan Wang (王圣男)
In-Reply-To: <20200518132731.20855-2-dongchun.zhu@mediatek.com>

Hi Dongchun,

On Mon, May 18, 2020 at 3:29 PM Dongchun Zhu <dongchun.zhu@mediatek.com> wrote:
>
> Add DeviceTree binding documentation for Dongwoon Anatech DW9768 voice
> coil actuator.

Thanks for the patch. Please see my comments below.

>
> Signed-off-by: Dongchun Zhu <dongchun.zhu@mediatek.com>
> Reviewed-by: Rob Herring <robh@kernel.org>

This version includes significant changes, so the reviewed-by tag
shouldn't have been carried out.

> ---
>  .../bindings/media/i2c/dongwoon,dw9768.yaml        | 105 +++++++++++++++++++++
>  MAINTAINERS                                        |   7 ++
>  2 files changed, 112 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/i2c/dongwoon,dw9768.yaml
>
> diff --git a/Documentation/devicetree/bindings/media/i2c/dongwoon,dw9768.yaml b/Documentation/devicetree/bindings/media/i2c/dongwoon,dw9768.yaml
> new file mode 100644
> index 0000000..b909e83
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/i2c/dongwoon,dw9768.yaml
> @@ -0,0 +1,105 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +# Copyright (c) 2020 MediaTek Inc.
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/media/i2c/dongwoon,dw9768.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Dongwoon Anatech DW9768 Voice Coil Motor (VCM) Lens Device Tree Bindings
> +
> +maintainers:
> +  - Dongchun Zhu <dongchun.zhu@mediatek.com>
> +
> +description: |-
> +  The Dongwoon DW9768 is a single 10-bit digital-to-analog (DAC) converter
> +  with 100 mA output current sink capability. VCM current is controlled with
> +  a linear mode driver. The DAC is controlled via a 2-wire (I2C-compatible)
> +  serial interface that operates at clock rates up to 1MHz. This chip
> +  integrates Advanced Actuator Control (AAC) technology and is intended for
> +  driving voice coil lenses in camera modules.
> +
> +properties:
> +  compatible:
> +    enum:
> +      # for DW9768 VCM
> +      - dongwoon,dw9768
> +      # for GT9769 VCM
> +      - giantec,gt9769
> +
> +  reg:
> +    maxItems: 1
> +
> +  vin-supply:
> +    description:
> +      Definition of the regulator used as I2C I/O interface power supply.
> +
> +  vdd-supply:
> +    description:
> +      Definition of the regulator used as VCM chip power supply.
> +
> +  dongwoon,aac-mode:
> +    description:
> +      Indication of AAC mode select.
> +    allOf:
> +      - $ref: "/schemas/types.yaml#/definitions/uint32"
> +      - enum:
> +          - 0    #  Direct (default)
> +          - 1    #  AAC2 (operation time# 0.48 x Tvib)
> +          - 2    #  AAC3 (operation time# 0.70 x Tvib)
> +          - 3    #  AAC4 (operation time# 0.75 x Tvib)
> +          - 4    #  Reserved
> +          - 5    #  AAC8 (operation time# 1.13 x Tvib)
> +          - 6    #  Reserved
> +          - 7    #  Reserved

I'll ultimately leave it to DT maintainers, but is there any reason to
define the reserved values?

> +
> +  dongwoon,aac-timing:
> +    description:
> +      Indication of AAC Timing count, unit of 0.1 milliseconds.
> +      Valid values vary from 0 to 63 (default 32).
> +    allOf:
> +      - $ref: "/schemas/types.yaml#/definitions/uint32"
> +
> +  dongwoon,clock-dividing-rate:
> +    description:
> +      Indication of VCM internal clock dividing rate select, as one multiple
> +      factor to calculate VCM ring periodic time Tvib.
> +    allOf:
> +      - $ref: "/schemas/types.yaml#/definitions/uint32"
> +      - enum:
> +          - 0    #  Dividing Rate -  2
> +          - 1    #  Dividing Rate -  1 (default)
> +          - 2    #  Dividing Rate -  1/2
> +          - 3    #  Dividing Rate -  1/4
> +          - 4    #  Dividing Rate -  8
> +          - 5    #  Dividing Rate -  4
> +          - 6    #  Dividing Rate -  Reserved
> +          - 7    #  Dividing Rate -  Reserved

Ditto.

Best regards,
Tomasz

^ permalink raw reply

* Re: [PATCH v3 10/15] net: ethernet: mtk-eth-mac: new driver
From: Bartosz Golaszewski @ 2020-05-18 14:07 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Jonathan Corbet, Rob Herring, David S . Miller, Matthias Brugger,
	John Crispin, Sean Wang, Mark Lee, Jakub Kicinski, Fabien Parent,
	Heiner Kallweit, Edwin Peer, DTML, linux-kernel@vger.kernel.org,
	Networking, Linux ARM, moderated list:ARM/Mediatek SoC...,
	Stephane Le Provost, Pedro Tsai, Andrew Perepech,
	Bartosz Golaszewski
In-Reply-To: <CAK8P3a0XgJtZNKePZUUpzADO25-JZKyDiVHFS_yuHRXTjvjDwg@mail.gmail.com>

pt., 15 maj 2020 o 15:32 Arnd Bergmann <arnd@arndb.de> napisał(a):
>
> On Thu, May 14, 2020 at 10:00 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> > +static int mtk_mac_ring_pop_tail(struct mtk_mac_ring *ring,
> > +                                struct mtk_mac_ring_desc_data *desc_data)
>
> I took another look at this function because of your comment on the locking
> the descriptor updates, which seemed suspicious as the device side does not
> actually use the locks to access them
>
> > +{
> > +       struct mtk_mac_ring_desc *desc = &ring->descs[ring->tail];
> > +       unsigned int status;
> > +
> > +       /* Let the device release the descriptor. */
> > +       dma_rmb();
> > +       status = desc->status;
> > +       if (!(status & MTK_MAC_DESC_BIT_COWN))
> > +               return -1;
>
> The dma_rmb() seems odd here, as I don't see which prior read
> is being protected by this.
>
> > +       desc_data->len = status & MTK_MAC_DESC_MSK_LEN;
> > +       desc_data->flags = status & ~MTK_MAC_DESC_MSK_LEN;
> > +       desc_data->dma_addr = ring->dma_addrs[ring->tail];
> > +       desc_data->skb = ring->skbs[ring->tail];
> > +
> > +       desc->data_ptr = 0;
> > +       desc->status = MTK_MAC_DESC_BIT_COWN;
> > +       if (status & MTK_MAC_DESC_BIT_EOR)
> > +               desc->status |= MTK_MAC_DESC_BIT_EOR;
> > +
> > +       /* Flush writes to descriptor memory. */
> > +       dma_wmb();
>
> The comment and the barrier here seem odd as well. I would have expected
> a barrier after the update to the data pointer, and only a single store
> but no read of the status flag instead of the read-modify-write,
> something like
>
>       desc->data_ptr = 0;
>       dma_wmb(); /* make pointer update visible before status update */
>       desc->status = MTK_MAC_DESC_BIT_COWN | (status & MTK_MAC_DESC_BIT_EOR);
>
> > +       ring->tail = (ring->tail + 1) % MTK_MAC_RING_NUM_DESCS;
> > +       ring->count--;
>
> I would get rid of the 'count' here, as it duplicates the information
> that is already known from the difference between head and tail, and you
> can't update it atomically without holding a lock around the access to
> the ring. The way I'd do this is to have the head and tail pointers
> in separate cache lines, and then use READ_ONCE/WRITE_ONCE
> and smp barriers to access them, with each one updated on one
> thread but read by the other.
>

Your previous solution seems much more reliable though. For instance
in the above: when we're doing the TX cleanup (we got the TX ready
irq, we're iterating over descriptors until we know there are no more
packets scheduled (count == 0) or we encounter one that's still owned
by DMA), a parallel TX path can schedule new packets to be sent and I
don't see how we can atomically check the count (understood as a
difference between tail and head) and run a new iteration (where we'd
modify the head or tail) without risking the other path getting in the
way. We'd have to always check the descriptor.

I experimented a bit with this and couldn't come up with anything that
would pass any stress test.

On the other hand: spin_lock_bh() works fine and I like your approach
from the previous e-mail - except for the work for updating stats as
we could potentially lose some stats when we're updating in process
context with RX/TX paths running in parallel in napi context but that
would be rare enough to overlook it.

I hope v4 will be good enough even with spinlocks. :)

Bart

^ permalink raw reply

* Re: [PATCH v3 7/7] clocksource: mips-gic-timer: Set limitations on clocksource/sched-clocks usage
From: Daniel Lezcano @ 2020-05-18 13:59 UTC (permalink / raw)
  To: Serge Semin
  Cc: Serge Semin, Thomas Bogendoerfer, Thomas Gleixner, Alexey Malahov,
	Paul Burton, Ralf Baechle, Alessandro Zummo, Alexandre Belloni,
	Arnd Bergmann, Rob Herring, linux-mips, linux-rtc, devicetree,
	Vincenzo Frascino, linux-kernel
In-Reply-To: <20200516121647.g6jua35kkihmw5r6@mobilestation>

On 16/05/2020 14:16, Serge Semin wrote:
> Hello Daniel,
> 
> Thanks for your comment. My response is below.
> 
> On Fri, May 15, 2020 at 07:10:04PM +0200, Daniel Lezcano wrote:
>> On Thu, May 07, 2020 at 12:41:07AM +0300, Serge Semin wrote:
>>> Currently neither clocksource nor scheduler clock kernel framework
>>> support the clocks with variable frequency. Needless to say how many
>>> problems may cause the sudden base clocks frequency change. In a
>>> simplest case the system time will either slow down or speed up.
>>> Since on CM2.5 and earlier MIPS GIC timer is synchronously clocked
>>> with CPU we must set some limitations on using it for these frameworks
>>> if CPU frequency may change. First of all it's not safe to have the
>>> MIPS GIC used for scheduler timings. So we shouldn't proceed with
>>> the clocks registration in the sched-subsystem. Secondly we must
>>> significantly decrease the MIPS GIC clocksource rating. This will let
>>> the system to use it only as a last resort.
>>>
>>> Note CM3.x-based systems may also experience the problems with MIPS GIC
>>> if the CPU-frequency change is activated for the whole CPU cluster
>>> instead of using the individual CPC core clocks divider.
>>
>> May be there is no alternative but the code looks a bit hacksih. Isn't possible
>> to do something with the sched_mark_unstable?
>>
>> Or just not use the timer at all ?
> 
> Not using the timer might be better, but not that good alternative either
> especially in our case due to very slow external timer. Me and Thomas
> Bogendoerfer discussed the similar commit I've provided to the csrc-r4k driver
> available on MIPS:
> https://lkml.org/lkml/2020/5/11/576
> 
> To cut it short, you are right. The solution with using clocksource_mark_unstable()
> is better alternative spied up in x86 tsc implementation. I'll use a similar
> approach here and submit the updated patch in v3.
> 
> Could you please proceed with the rest of the series review? I'd like to send
> the next version with as many comments taken into account as possible. The
> patchset has been submitted a while ago, but except Rob noone have had any
> comments.(

For me other patches are ok.

I can apply patches 1, 2, 4, 5, 6

Will remain patches 3 et 7


-- 
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

^ permalink raw reply

* Re: [PATCH v4 00/12] PCI: aardvark: Fix support for Turris MOX and Compex wifi cards
From: Marek Behun @ 2020-05-18 13:50 UTC (permalink / raw)
  To: Lorenzo Pieralisi
  Cc: Pali Rohár, Gregory CLEMENT, Andrew Murray, Bjorn Helgaas,
	Thomas Petazzoni, Jason Cooper, Andrew Lunn,
	Sebastian Hesselbarth, Rob Herring, Remi Pommarel,
	Tomasz Maciej Nowak, Xogium, devicetree, linux-kernel, linux-pci
In-Reply-To: <20200518134614.GA31554@e121166-lin.cambridge.arm.com>

On Mon, 18 May 2020 14:46:14 +0100
Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> wrote:

> On Mon, May 18, 2020 at 12:30:04PM +0200, Pali Rohár wrote:
> > On Sunday 17 May 2020 17:57:02 Gregory CLEMENT wrote:  
> > > Hello,
> > >   
> > > > Hello,
> > > >
> > > > On Thu, 30 Apr 2020 10:06:13 +0200
> > > > Pali Rohár <pali@kernel.org> wrote:
> > > >  
> > > >> Marek Behún (5):
> > > >>   PCI: aardvark: Improve link training
> > > >>   PCI: aardvark: Add PHY support
> > > >>   dt-bindings: PCI: aardvark: Describe new properties
> > > >>   arm64: dts: marvell: armada-37xx: Set pcie_reset_pin to gpio function
> > > >>   arm64: dts: marvell: armada-37xx: Move PCIe comphy handle property
> > > >> 
> > > >> Pali Rohár (7):
> > > >>   PCI: aardvark: Train link immediately after enabling training
> > > >>   PCI: aardvark: Don't blindly enable ASPM L0s and don't write to
> > > >>     read-only register
> > > >>   PCI: of: Zero max-link-speed value is invalid
> > > >>   PCI: aardvark: Issue PERST via GPIO
> > > >>   PCI: aardvark: Add FIXME comment for PCIE_CORE_CMD_STATUS_REG access
> > > >>   PCI: aardvark: Replace custom macros by standard linux/pci_regs.h
> > > >>     macros
> > > >>   arm64: dts: marvell: armada-37xx: Move PCIe max-link-speed property  
> > > >
> > > > Thanks a lot for this work. For a number of reasons, I'm less involved
> > > > in Marvell platform support in Linux, but I reviewed your series and
> > > > followed the discussions around it, and I'm happy to give my:
> > > >
> > > > Acked-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>  
> > > 
> > > With this acked-by for the series, the reviewed-by from Rob on the
> > > binding and the tested-by, I am pretty confident so I applied the
> > > patches 10, 11 and 12 on mvebu/dt64.
> > > 
> > > Thanks,
> > > 
> > > Gregory  
> > 
> > Thank you!
> > 
> > Lorenzo, would you now take remaining patches?  
> 
> Yes - even though I have reservations about patch (5) and the
> problem is related to a complete lack of programming model for
> these host controllers and a clear separation between what's
> done in the OS vs bootloader, PERST handling in this host
> bridge is *really* a mess.
> 
> I applied 1-9 to pci/aardvark.
> 
> Lorenzo

Hooray, thanks, Lorenzo (and everyone else).

^ permalink raw reply

* Re: [RESEND PATCH v8 0/3] Add Intel ComboPhy driver
From: Kishon Vijay Abraham I @ 2020-05-18 13:49 UTC (permalink / raw)
  To: Dilip Kota, linux-kernel, vkoul, devicetree
  Cc: robh, andriy.shevchenko, cheol.yong.kim, chuanhua.lei, qi-ming.wu,
	yixin.zhu
In-Reply-To: <cover.1589530082.git.eswara.kota@linux.intel.com>

Dilip,

On 5/15/2020 1:43 PM, Dilip Kota wrote:
> This patch series adds Intel ComboPhy driver, respective yaml schemas
> 
> Changes on v8:
>   As per PHY Maintainer's request add description in comments for doing
>   register access through register map framework.
> 
> Changes on v7:
>   As per System control driver maintainer's inputs remove
>     fwnode_to_regmap() definition and use device_node_get_regmap()

Can you fix this warning and resend the patch?
drivers/phy/intel/phy-intel-combo.c:229:6: warning: ‘cb_mode’ may be used
uninitialized in this function [-Wmaybe-uninitialized]
  ret = regmap_write(cbphy->hsiocfg, REG_COMBO_MODE(cbphy->bid), cb_mode);
  ~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/phy/intel/phy-intel-combo.c:204:24: note: ‘cb_mode’ was declared here
  enum intel_combo_mode cb_mode;
                        ^~~~~~~

Thanks
Kishon
>     
> Changes on v6:
>   Rebase patches on the latest maintainer's branch
>   https://git.kernel.org/pub/scm/linux/kernel/git/kishon/linux-phy.git/?h=phy-for-5.7
> Dilip Kota (3):
>   dt-bindings: phy: Add PHY_TYPE_XPCS definition
>   dt-bindings: phy: Add YAML schemas for Intel ComboPhy
>   phy: intel: Add driver support for ComboPhy
> 
>  .../devicetree/bindings/phy/intel,combo-phy.yaml   | 101 ++++
>  drivers/phy/intel/Kconfig                          |  14 +
>  drivers/phy/intel/Makefile                         |   1 +
>  drivers/phy/intel/phy-intel-combo.c                | 632 +++++++++++++++++++++
>  include/dt-bindings/phy/phy.h                      |   1 +
>  5 files changed, 749 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/phy/intel,combo-phy.yaml
>  create mode 100644 drivers/phy/intel/phy-intel-combo.c
> 

^ permalink raw reply

* Re: [PATCH v2 18/20] mips: csrc-r4k: Decrease r4k-clocksource rating if CPU_FREQ enabled
From: Serge Semin @ 2020-05-18 13:48 UTC (permalink / raw)
  To: Thomas Bogendoerfer
  Cc: Serge Semin, Alexey Malahov, Paul Burton, Ralf Baechle,
	Greg Kroah-Hartman, Arnd Bergmann, Rob Herring, linux-pm,
	devicetree, Vincenzo Frascino, Thomas Gleixner, linux-mips,
	linux-kernel
In-Reply-To: <20200515210647.GA22922@alpha.franken.de>

On Fri, May 15, 2020 at 11:06:47PM +0200, Thomas Bogendoerfer wrote:
> On Fri, May 15, 2020 at 10:48:27AM +0300, Serge Semin wrote:
> > Thomas,
> > Could you take a look at my comment below so I could proceed with the
> > patchset v3 development?
> 
> I can't help, but using r4k clocksource with changing frequency is
> probaly only usefull as a random generator. So IMHO the only two
> options are disabling it or implement what arch/x86/kernel/tsc.c does.
> 
> Thomas.

Thomas, could you proceed with the rest of the patches review?
├─>[PATCH v2 16/20] bus: cdmm: Add MIPS R5 arch support
├─>[PATCH v2 15/20] mips: cdmm: Add mti,mips-cdmm dtb node support
├─>[PATCH v2 13/20] mips: early_printk_8250: Use offset-sized IO-mem accessors
├─>[PATCH v2 12/20] mips: MAAR: Add XPA mode support
├─>[PATCH v2 10/20] mips: Add CONFIG/CONFIG6/Cause reg fields macro
└─>[PATCH v2 09/20] mips: Add CP0 Write Merge config support

It would be great if I fixed more comments in the next patchset version.

-Sergey

> 
> -- 
> Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
> good idea.                                                [ RFC1925, 2.3 ]

^ permalink raw reply

* Re: [PATCH v4 00/12] PCI: aardvark: Fix support for Turris MOX and Compex wifi cards
From: Lorenzo Pieralisi @ 2020-05-18 13:46 UTC (permalink / raw)
  To: Pali Rohár
  Cc: Gregory CLEMENT, Andrew Murray, Bjorn Helgaas, Thomas Petazzoni,
	Jason Cooper, Andrew Lunn, Sebastian Hesselbarth, Rob Herring,
	Remi Pommarel, Marek Behún, Tomasz Maciej Nowak, Xogium,
	devicetree, linux-kernel, linux-pci
In-Reply-To: <20200518103004.6tydnad3apkfn77y@pali>

On Mon, May 18, 2020 at 12:30:04PM +0200, Pali Rohár wrote:
> On Sunday 17 May 2020 17:57:02 Gregory CLEMENT wrote:
> > Hello,
> > 
> > > Hello,
> > >
> > > On Thu, 30 Apr 2020 10:06:13 +0200
> > > Pali Rohár <pali@kernel.org> wrote:
> > >
> > >> Marek Behún (5):
> > >>   PCI: aardvark: Improve link training
> > >>   PCI: aardvark: Add PHY support
> > >>   dt-bindings: PCI: aardvark: Describe new properties
> > >>   arm64: dts: marvell: armada-37xx: Set pcie_reset_pin to gpio function
> > >>   arm64: dts: marvell: armada-37xx: Move PCIe comphy handle property
> > >> 
> > >> Pali Rohár (7):
> > >>   PCI: aardvark: Train link immediately after enabling training
> > >>   PCI: aardvark: Don't blindly enable ASPM L0s and don't write to
> > >>     read-only register
> > >>   PCI: of: Zero max-link-speed value is invalid
> > >>   PCI: aardvark: Issue PERST via GPIO
> > >>   PCI: aardvark: Add FIXME comment for PCIE_CORE_CMD_STATUS_REG access
> > >>   PCI: aardvark: Replace custom macros by standard linux/pci_regs.h
> > >>     macros
> > >>   arm64: dts: marvell: armada-37xx: Move PCIe max-link-speed property
> > >
> > > Thanks a lot for this work. For a number of reasons, I'm less involved
> > > in Marvell platform support in Linux, but I reviewed your series and
> > > followed the discussions around it, and I'm happy to give my:
> > >
> > > Acked-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> > 
> > With this acked-by for the series, the reviewed-by from Rob on the
> > binding and the tested-by, I am pretty confident so I applied the
> > patches 10, 11 and 12 on mvebu/dt64.
> > 
> > Thanks,
> > 
> > Gregory
> 
> Thank you!
> 
> Lorenzo, would you now take remaining patches?

Yes - even though I have reservations about patch (5) and the
problem is related to a complete lack of programming model for
these host controllers and a clear separation between what's
done in the OS vs bootloader, PERST handling in this host
bridge is *really* a mess.

I applied 1-9 to pci/aardvark.

Lorenzo

^ permalink raw reply

* Re: [PATCH v2 10/19] spi: dw: Use DMA max burst to set the request thresholds
From: Serge Semin @ 2020-05-18 13:43 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Serge Semin, Andy Shevchenko, Mark Brown, Alexey Malahov,
	Thomas Bogendoerfer, Paul Burton, Ralf Baechle, Arnd Bergmann,
	Allison Randal, Gareth Williams, Rob Herring, linux-mips,
	devicetree, Georgy Vlasov, Ramil Zaripov, Jarkko Nikula,
	Thomas Gleixner, Wan Ahmad Zainie, Linus Walleij, Clement Leger,
	linux-spi, Linux Kernel Mailing List
In-Reply-To: <CAHp75VeMcv-hQViCANQARiNh0LwmugsDWk=MF1c5E3t7z5h02Q@mail.gmail.com>

On Mon, May 18, 2020 at 04:25:20PM +0300, Andy Shevchenko wrote:
> On Mon, May 18, 2020 at 3:53 PM Serge Semin
> <Sergey.Semin@baikalelectronics.ru> wrote:
> > On Mon, May 18, 2020 at 02:03:43PM +0300, Andy Shevchenko wrote:
> > > On Sat, May 16, 2020 at 11:01:33PM +0300, Serge Semin wrote:
> > > > On Fri, May 15, 2020 at 05:38:42PM +0300, Andy Shevchenko wrote:
> > > > > On Fri, May 15, 2020 at 01:47:49PM +0300, Serge Semin wrote:
> 
> ...
> 
> > > > > >         struct dma_chan         *txchan;
> > > > > > +       u32                     txburst;
> > > > > >         struct dma_chan         *rxchan;
> > > > > > +       u32                     rxburst;
> > > > >
> > > > > Leave u32 together, it may be optimal on 64-bit architectures where ABIs require padding.
> > > >
> > > > It's not like anyone cared about padding in this structure in the first place)
> > >
> > > I think I have been caring (to some extend).
> >
> > Well, If you have then instead of asking to rearrange just two members (which
> > by the way finely grouped by the Tx-Rx affiliation) why not sending a
> > patch, which would refactor the whole structure so to be optimal for the x64
> > platforms? I don't really see why this gets very important for you seeing
> > Mark is Ok with this. My current commit follows the common driver design
> > including the DW SSI data members grouping. On the second thought I'll leave
> > it as is then.
> 
> Again same issue here. What is really easy to do for you here, will
> become a burden and additional churn to anybody else.
> So, why not to minimize it in the first place? Same with comma in
> another patch. Sorry, I really don't get it.

If comma is more or less understandable (though adding it is absolutely
redundant there and doesn't worth even a bit of time spending for the
discussion), here you consider the patch from padding point of view.
The driver developer didn't care about it, but did care about grouping the
members in a corresponding way. The padding burden will be there anyway and
should be fixed for the whole structure in an additional patch. Until then
the way of grouping should be preserved.

-Sergey

> 
> -- 
> With Best Regards,
> Andy Shevchenko

^ permalink raw reply

* [PATCH 3/3] iio: magnetometer: ak8975: Add gpio reset support
From: Jonathan Albrieux @ 2020-05-18 13:36 UTC (permalink / raw)
  To: linux-kernel
  Cc: Jonathan Albrieux, Andy Shevchenko,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Greg Kroah-Hartman, Hartmut Knaack, Jonathan Cameron,
	Lars-Peter Clausen, Linus Walleij,
	open list:IIO SUBSYSTEM AND DRIVERS, Peter Meerwald-Stadler,
	Steve Winslow, Thomas Gleixner, Jonathan Cameron
In-Reply-To: <20200518133645.19127-1-jonathan.albrieux@gmail.com>

Set reset gpio to high on ak8975_power_on, set reset gpio to low on
ak8975_power_off.

Without setting it to high on ak8975_power_on driver's probe fails
on ak8975_who_i_am while checking for device identity for AK09911 chip

AK09911 has a reset gpio to handle register's reset. If reset gpio is
set to low it will trigger the reset. AK09911 datasheed says that if not
used reset pin should be connected to VID and this patch emulates this
situation

Signed-off-by: Jonathan Albrieux <jonathan.albrieux@gmail.com>
---
 drivers/iio/magnetometer/ak8975.c | 21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/magnetometer/ak8975.c b/drivers/iio/magnetometer/ak8975.c
index 3c881541ae72..c1ba5cd2cb6c 100644
--- a/drivers/iio/magnetometer/ak8975.c
+++ b/drivers/iio/magnetometer/ak8975.c
@@ -358,6 +358,7 @@ struct ak8975_data {
 	u8			asa[3];
 	long			raw_to_gauss[3];
 	struct gpio_desc	*eoc_gpiod;
+	struct gpio_desc	*reset_gpiod;
 	int			eoc_irq;
 	wait_queue_head_t	data_ready_queue;
 	unsigned long		flags;
@@ -384,10 +385,13 @@ static int ak8975_power_on(const struct ak8975_data *data)
 			 "Failed to enable specified Vid supply\n");
 		return ret;
 	}
+
+	gpiod_set_value_cansleep(data->reset_gpiod, 0);
+
 	/*
-	 * According to the datasheet the power supply rise time i 200us
+	 * According to the datasheet the power supply rise time is 200us
 	 * and the minimum wait time before mode setting is 100us, in
-	 * total 300 us. Add some margin and say minimum 500us here.
+	 * total 300us. Add some margin and say minimum 500us here.
 	 */
 	usleep_range(500, 1000);
 	return 0;
@@ -396,6 +400,8 @@ static int ak8975_power_on(const struct ak8975_data *data)
 /* Disable attached power regulator if any. */
 static void ak8975_power_off(const struct ak8975_data *data)
 {
+	gpiod_set_value_cansleep(data->reset_gpiod, 1);
+
 	regulator_disable(data->vid);
 	regulator_disable(data->vdd);
 }
@@ -839,6 +845,7 @@ static int ak8975_probe(struct i2c_client *client,
 	struct ak8975_data *data;
 	struct iio_dev *indio_dev;
 	struct gpio_desc *eoc_gpiod;
+	struct gpio_desc *reset_gpiod;
 	const void *match;
 	unsigned int i;
 	int err;
@@ -856,6 +863,15 @@ static int ak8975_probe(struct i2c_client *client,
 	if (eoc_gpiod)
 		gpiod_set_consumer_name(eoc_gpiod, "ak_8975");
 
+	/*
+	 * If reset pin is provided then will be set to high on power on
+	 * and to low on power off according to AK09911 datasheet
+	 */
+	reset_gpiod = devm_gpiod_get_optional(&client->dev,
+					      "reset", GPIOD_OUT_HIGH);
+	if (IS_ERR(reset_gpiod))
+		return PTR_ERR(reset_gpiod);
+
 	/* Register with IIO */
 	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
 	if (indio_dev == NULL)
@@ -866,6 +882,7 @@ static int ak8975_probe(struct i2c_client *client,
 
 	data->client = client;
 	data->eoc_gpiod = eoc_gpiod;
+	data->reset_gpiod = reset_gpiod;
 	data->eoc_irq = 0;
 
 	err = iio_read_mount_matrix(&client->dev, "mount-matrix", &data->orientation);
-- 
2.17.1


^ permalink raw reply related

* [PATCH 2/3] dt-bindings: iio: magnetometer: ak8975: add gpio reset support
From: Jonathan Albrieux @ 2020-05-18 13:36 UTC (permalink / raw)
  To: linux-kernel
  Cc: Jonathan Albrieux, Andy Shevchenko,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Greg Kroah-Hartman, Hartmut Knaack, Jonathan Cameron,
	Lars-Peter Clausen, Linus Walleij,
	open list:IIO SUBSYSTEM AND DRIVERS, Peter Meerwald-Stadler,
	Steve Winslow, Thomas Gleixner, Jonathan Cameron, Rob Herring
In-Reply-To: <20200518133645.19127-1-jonathan.albrieux@gmail.com>

Add reset-gpio support.

AK09911 has a reset gpio to handle register's reset. If reset gpio is
set to low it will trigger the reset. AK09911 datasheed says that if not
used reset pin should be connected to VID and this patch emulates this
situation

Without setting it to high on ak8975_power_on driver's probe fails
on ak8975_who_i_am while checking for device identity for AK09911 chip

Signed-off-by: Jonathan Albrieux <jonathan.albrieux@gmail.com>
---
 .../devicetree/bindings/iio/magnetometer/ak8975.yaml          | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/iio/magnetometer/ak8975.yaml b/Documentation/devicetree/bindings/iio/magnetometer/ak8975.yaml
index 86e3efa693a8..a82c0ff5d098 100644
--- a/Documentation/devicetree/bindings/iio/magnetometer/ak8975.yaml
+++ b/Documentation/devicetree/bindings/iio/magnetometer/ak8975.yaml
@@ -37,6 +37,9 @@ properties:
   mount-matrix:
     description: an optional 3x3 mounting rotation matrix
 
+  reset-gpio:
+    description: an optional pin needed for AK09911 to set the reset state
+
 required:
   - compatible
   - reg
@@ -53,6 +56,7 @@ examples:
             reg = <0x0c>;
             gpios = <&gpj0 7 0>;
             vdd-supply = <&ldo_3v3_gnss>;
+            reset-gpio = <&msmgpio 111 1>;
             mount-matrix = "-0.984807753012208",  /* x0 */
                            "0",                   /* y0 */
                            "-0.173648177666930",  /* z0 */
-- 
2.17.1


^ permalink raw reply related

* [PATCH 1/3] dt-bindings: iio: magnetometer: ak8975: convert txt format to yaml
From: Jonathan Albrieux @ 2020-05-18 13:36 UTC (permalink / raw)
  To: linux-kernel
  Cc: Jonathan Albrieux, Andy Shevchenko,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Greg Kroah-Hartman, Hartmut Knaack, Jonathan Cameron,
	Lars-Peter Clausen, Linus Walleij,
	open list:IIO SUBSYSTEM AND DRIVERS, Peter Meerwald-Stadler,
	Steve Winslow, Thomas Gleixner, Jonathan Cameron, Rob Herring
In-Reply-To: <20200518133645.19127-1-jonathan.albrieux@gmail.com>

Converts documentation from txt format to yaml

Signed-off-by: Jonathan Albrieux <jonathan.albrieux@gmail.com>
---
 .../bindings/iio/magnetometer/ak8975.txt      | 30 ---------
 .../bindings/iio/magnetometer/ak8975.yaml     | 66 +++++++++++++++++++
 2 files changed, 66 insertions(+), 30 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/iio/magnetometer/ak8975.txt
 create mode 100644 Documentation/devicetree/bindings/iio/magnetometer/ak8975.yaml

diff --git a/Documentation/devicetree/bindings/iio/magnetometer/ak8975.txt b/Documentation/devicetree/bindings/iio/magnetometer/ak8975.txt
deleted file mode 100644
index aa67ceb0d4e0..000000000000
--- a/Documentation/devicetree/bindings/iio/magnetometer/ak8975.txt
+++ /dev/null
@@ -1,30 +0,0 @@
-* AsahiKASEI AK8975 magnetometer sensor
-
-Required properties:
-
-  - compatible : should be "asahi-kasei,ak8975"
-  - reg : the I2C address of the magnetometer
-
-Optional properties:
-
-  - gpios : should be device tree identifier of the magnetometer DRDY pin
-  - vdd-supply: an optional regulator that needs to be on to provide VDD
-  - mount-matrix: an optional 3x3 mounting rotation matrix
-
-Example:
-
-ak8975@c {
-        compatible = "asahi-kasei,ak8975";
-        reg = <0x0c>;
-        gpios = <&gpj0 7 0>;
-        vdd-supply = <&ldo_3v3_gnss>;
-        mount-matrix = "-0.984807753012208",  /* x0 */
-                       "0",                   /* y0 */
-                       "-0.173648177666930",  /* z0 */
-                       "0",                   /* x1 */
-                       "-1",                  /* y1 */
-                       "0",                   /* z1 */
-                       "-0.173648177666930",  /* x2 */
-                       "0",                   /* y2 */
-                       "0.984807753012208";   /* z2 */
-};
diff --git a/Documentation/devicetree/bindings/iio/magnetometer/ak8975.yaml b/Documentation/devicetree/bindings/iio/magnetometer/ak8975.yaml
new file mode 100644
index 000000000000..86e3efa693a8
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/magnetometer/ak8975.yaml
@@ -0,0 +1,66 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/magnetometer/ak8975.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: AsahiKASEI AK8975 magnetometer sensor
+
+maintainers:
+  - can't find a mantainer, author is Laxman Dewangan <ldewangan@nvidia.com>
+
+properties:
+  compatible:
+    enum:
+      - "asahi-kasei,ak8975"
+      - "ak8975"
+      - "asahi-kasei,ak8963"
+      - "ak8963"
+      - "asahi-kasei,ak09911"
+      - "ak09911"
+      - "asahi-kasei,ak09912"
+      - "ak09912"
+
+  reg:
+    maxItems: 1
+    description: the I2C address of the magnetometer
+
+  gpios:
+    description: should be device tree identifier of the magnetometer DRDY pin
+
+  vdd-supply:
+    maxItems: 1
+    description: |
+      an optional regulator that needs to be on to provide VDD power to
+      the sensor.
+
+  mount-matrix:
+    description: an optional 3x3 mounting rotation matrix
+
+required:
+  - compatible
+  - reg
+
+examples:
+  - |
+    i2c@78b7000 {
+        reg = <0x78b6000 0x600>;
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        ak8975@c {
+            compatible = "asahi-kasei,ak8975";
+            reg = <0x0c>;
+            gpios = <&gpj0 7 0>;
+            vdd-supply = <&ldo_3v3_gnss>;
+            mount-matrix = "-0.984807753012208",  /* x0 */
+                           "0",                   /* y0 */
+                           "-0.173648177666930",  /* z0 */
+                           "0",                   /* x1 */
+                           "-1",                  /* y1 */
+                           "0",                   /* z1 */
+                           "-0.173648177666930",  /* x2 */
+                           "0",                   /* y2 */
+                           "0.984807753012208";   /* z2 */
+        };
+    };
-- 
2.17.1


^ permalink raw reply related

* [PATCH 0/3] iio: magnetometer: ak8975: Add gpio reset support
From: Jonathan Albrieux @ 2020-05-18 13:36 UTC (permalink / raw)
  To: linux-kernel
  Cc: Jonathan Albrieux, Andy Shevchenko,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Greg Kroah-Hartman, Hartmut Knaack, Jonathan Cameron,
	Lars-Peter Clausen, Linus Walleij,
	open list:IIO SUBSYSTEM AND DRIVERS, Peter Meerwald-Stadler,
	Steve Winslow, Thomas Gleixner

Convert documentation from txt format to yaml. Add documentation about
reset-gpio. 

Set reset gpio to high on ak8975_power_on, set reset gpio to low on
ak8975_power_off.

Without setting it to high on ak8975_power_on driver's probe fails
on ak8975_who_i_am while checking for device identity for AK09911 chip

AK09911 has a reset gpio to handle register's reset. If reset gpio is
set to low it will trigger the reset. AK09911 datasheed says that if not
used reset pin should be connected to VID and this patch emulates this
situation

Jonathan Albrieux (3):
  dt-bindings: iio: magnetometer: ak8975: convert txt format to yaml
  dt-bindings: iio: magnetometer: ak8975: add gpio reset support
  iio: magnetometer: ak8975: Add gpio reset support

 .../bindings/iio/magnetometer/ak8975.txt      | 30 --------
 .../bindings/iio/magnetometer/ak8975.yaml     | 70 +++++++++++++++++++
 drivers/iio/magnetometer/ak8975.c             | 21 +++++-
 3 files changed, 89 insertions(+), 32 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/iio/magnetometer/ak8975.txt
 create mode 100644 Documentation/devicetree/bindings/iio/magnetometer/ak8975.yaml

-- 
2.17.1


^ permalink raw reply

* [PATCH 4/4] iio: imu: bmi160: added mount-matrix support
From: Jonathan Albrieux @ 2020-05-18 13:33 UTC (permalink / raw)
  To: linux-kernel
  Cc: Jonathan Albrieux,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Hartmut Knaack, Lars-Peter Clausen,
	open list:IIO SUBSYSTEM AND DRIVERS, Peter Meerwald-Stadler,
	Jonathan Cameron
In-Reply-To: <20200518133358.18978-1-jonathan.albrieux@gmail.com>

Add mount-matrix binding support. As chip could have different orientations
a mount matrix support is needed to correctly translate these differences

Signed-off-by: Jonathan Albrieux <jonathan.albrieux@gmail.com>
---
 drivers/iio/imu/bmi160/bmi160.h      |  1 +
 drivers/iio/imu/bmi160/bmi160_core.c | 20 ++++++++++++++++++++
 2 files changed, 21 insertions(+)

diff --git a/drivers/iio/imu/bmi160/bmi160.h b/drivers/iio/imu/bmi160/bmi160.h
index 923c3b274fde..a82e040bd109 100644
--- a/drivers/iio/imu/bmi160/bmi160.h
+++ b/drivers/iio/imu/bmi160/bmi160.h
@@ -9,6 +9,7 @@ struct bmi160_data {
 	struct regmap *regmap;
 	struct iio_trigger *trig;
 	struct regulator_bulk_data supplies[2];
+	struct iio_mount_matrix orientation;
 };
 
 extern const struct regmap_config bmi160_regmap_config;
diff --git a/drivers/iio/imu/bmi160/bmi160_core.c b/drivers/iio/imu/bmi160/bmi160_core.c
index 9bbe0d8e6720..78c8ca962359 100644
--- a/drivers/iio/imu/bmi160/bmi160_core.c
+++ b/drivers/iio/imu/bmi160/bmi160_core.c
@@ -110,6 +110,7 @@
 		.storagebits = 16,				\
 		.endianness = IIO_LE,				\
 	},							\
+	.ext_info = bmi160_ext_info,				\
 }
 
 /* scan indexes follow DATA register order */
@@ -265,6 +266,20 @@ static const struct  bmi160_odr_item bmi160_odr_table[] = {
 	},
 };
 
+static const struct iio_mount_matrix *
+bmi160_get_mount_matrix(const struct iio_dev *indio_dev,
+			const struct iio_chan_spec *chan)
+{
+	struct bmi160_data *data = iio_priv(indio_dev);
+
+	return &data->orientation;
+}
+
+static const struct iio_chan_spec_ext_info bmi160_ext_info[] = {
+	IIO_MOUNT_MATRIX(IIO_SHARED_BY_DIR, bmi160_get_mount_matrix),
+	{ }
+};
+
 static const struct iio_chan_spec bmi160_channels[] = {
 	BMI160_CHANNEL(IIO_ACCEL, X, BMI160_SCAN_ACCEL_X),
 	BMI160_CHANNEL(IIO_ACCEL, Y, BMI160_SCAN_ACCEL_Y),
@@ -840,6 +855,11 @@ int bmi160_core_probe(struct device *dev, struct regmap *regmap,
 		return ret;
 	}
 
+	ret = iio_read_mount_matrix(dev, "mount-matrix",
+				    &data->orientation);
+	if (ret)
+		return ret;
+
 	ret = bmi160_chip_init(data, use_spi);
 	if (ret)
 		return ret;
-- 
2.17.1


^ permalink raw reply related

* [PATCH 3/4] iio: imu: bmi160: added regulator support
From: Jonathan Albrieux @ 2020-05-18 13:33 UTC (permalink / raw)
  To: linux-kernel
  Cc: Jonathan Albrieux,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Hartmut Knaack, Lars-Peter Clausen,
	open list:IIO SUBSYSTEM AND DRIVERS, Peter Meerwald-Stadler,
	Jonathan Cameron
In-Reply-To: <20200518133358.18978-1-jonathan.albrieux@gmail.com>

---
 drivers/iio/imu/bmi160/bmi160.h      |  2 ++
 drivers/iio/imu/bmi160/bmi160_core.c | 27 ++++++++++++++++++++++++++-
 2 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/imu/bmi160/bmi160.h b/drivers/iio/imu/bmi160/bmi160.h
index 621f5309d735..923c3b274fde 100644
--- a/drivers/iio/imu/bmi160/bmi160.h
+++ b/drivers/iio/imu/bmi160/bmi160.h
@@ -3,10 +3,12 @@
 #define BMI160_H_
 
 #include <linux/iio/iio.h>
+#include <linux/regulator/consumer.h>
 
 struct bmi160_data {
 	struct regmap *regmap;
 	struct iio_trigger *trig;
+	struct regulator_bulk_data supplies[2];
 };
 
 extern const struct regmap_config bmi160_regmap_config;
diff --git a/drivers/iio/imu/bmi160/bmi160_core.c b/drivers/iio/imu/bmi160/bmi160_core.c
index 6af65d6f1d28..9bbe0d8e6720 100644
--- a/drivers/iio/imu/bmi160/bmi160_core.c
+++ b/drivers/iio/imu/bmi160/bmi160_core.c
@@ -15,6 +15,7 @@
 #include <linux/delay.h>
 #include <linux/irq.h>
 #include <linux/of_irq.h>
+#include <linux/regulator/consumer.h>
 
 #include <linux/iio/iio.h>
 #include <linux/iio/triggered_buffer.h>
@@ -709,6 +710,12 @@ static int bmi160_chip_init(struct bmi160_data *data, bool use_spi)
 	unsigned int val;
 	struct device *dev = regmap_get_device(data->regmap);
 
+	ret = regulator_bulk_enable(ARRAY_SIZE(data->supplies), data->supplies);
+	if (ret) {
+		dev_err(dev, "Failed to enable regulators: %d\n", ret);
+		return ret;
+	}
+
 	ret = regmap_write(data->regmap, BMI160_REG_CMD, BMI160_CMD_SOFTRESET);
 	if (ret)
 		return ret;
@@ -793,9 +800,17 @@ int bmi160_probe_trigger(struct iio_dev *indio_dev, int irq, u32 irq_type)
 static void bmi160_chip_uninit(void *data)
 {
 	struct bmi160_data *bmi_data = data;
+	struct device *dev = regmap_get_device(bmi_data->regmap);
+	int ret;
 
 	bmi160_set_mode(bmi_data, BMI160_GYRO, false);
 	bmi160_set_mode(bmi_data, BMI160_ACCEL, false);
+
+	ret = regulator_bulk_disable(ARRAY_SIZE(bmi_data->supplies),
+				     bmi_data->supplies);
+	if (ret) {
+		dev_err(dev, "Failed to disable regulators: %d\n", ret);
+	}
 }
 
 int bmi160_core_probe(struct device *dev, struct regmap *regmap,
@@ -815,6 +830,16 @@ int bmi160_core_probe(struct device *dev, struct regmap *regmap,
 	dev_set_drvdata(dev, indio_dev);
 	data->regmap = regmap;
 
+	data->supplies[0].supply = "vdd";
+	data->supplies[1].supply = "vddio";
+	ret = devm_regulator_bulk_get(dev,
+				      ARRAY_SIZE(data->supplies),
+				      data->supplies);
+	if (ret) {
+		dev_err(dev, "Failed to get regulators: %d\n", ret);
+		return ret;
+	}
+
 	ret = bmi160_chip_init(data, use_spi);
 	if (ret)
 		return ret;
@@ -853,6 +878,6 @@ int bmi160_core_probe(struct device *dev, struct regmap *regmap,
 }
 EXPORT_SYMBOL_GPL(bmi160_core_probe);
 
-MODULE_AUTHOR("Daniel Baluta <daniel.baluta@intel.com");
+MODULE_AUTHOR("Daniel Baluta <daniel.baluta@intel.com>");
 MODULE_DESCRIPTION("Bosch BMI160 driver");
 MODULE_LICENSE("GPL v2");
-- 
2.17.1


^ permalink raw reply related

* [PATCH 2/4] dt-bindings: iio: imu: bmi160: add regulators and mount-matrix
From: Jonathan Albrieux @ 2020-05-18 13:33 UTC (permalink / raw)
  To: linux-kernel
  Cc: Jonathan Albrieux,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Hartmut Knaack, Lars-Peter Clausen,
	open list:IIO SUBSYSTEM AND DRIVERS, Peter Meerwald-Stadler,
	Jonathan Cameron, Rob Herring
In-Reply-To: <20200518133358.18978-1-jonathan.albrieux@gmail.com>

Add vdd-supply and vddio-supply support.
Add mount-matrix support.

Signed-off-by: Jonathan Albrieux <jonathan.albrieux@gmail.com>
---
 .../devicetree/bindings/iio/imu/bmi160.yaml   | 21 +++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/Documentation/devicetree/bindings/iio/imu/bmi160.yaml b/Documentation/devicetree/bindings/iio/imu/bmi160.yaml
index 6b464ce5ed0b..5b13af7a209f 100644
--- a/Documentation/devicetree/bindings/iio/imu/bmi160.yaml
+++ b/Documentation/devicetree/bindings/iio/imu/bmi160.yaml
@@ -46,6 +46,21 @@ properties:
       set if the specified interrupt pin should be configured as
       open drain. If not set, defaults to push-pull.
 
+  vdd-supply:
+    maxItems: 1
+    description: |
+      an optional regulator that needs to be on to provide VDD power to
+      the sensor.
+
+  vddio-supply:
+    maxItems: 1
+    description: |
+      an optional regulator that needs to be on to provide the VDD IO power to
+      the sensor.
+
+  mount-matrix:
+    description: an optional 3x3 mounting rotation matrix
+
 required:
   - compatible
   - reg
@@ -61,9 +76,15 @@ examples:
         bmi160@68 {
                 compatible = "bosch,bmi160";
                 reg = <0x68>;
+                vdd-supply = <&pm8916_l17>;
+                vddio-supply = <&pm8916_l6>;
                 interrupt-parent = <&gpio4>;
                 interrupts = <12 1>;
                 interrupt-names = "INT1";
+                mount-matrix = "0", "1", "0",
+                               "-1", "0", "0",
+                               "0", "0", "1";
+                };
         };
   - |
     // Example for SPI
-- 
2.17.1


^ permalink raw reply related

* [PATCH 1/4] dt-bindings: iio: imu: bmi160: convert txt format to yaml
From: Jonathan Albrieux @ 2020-05-18 13:33 UTC (permalink / raw)
  To: linux-kernel
  Cc: Jonathan Albrieux,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Hartmut Knaack, Lars-Peter Clausen,
	open list:IIO SUBSYSTEM AND DRIVERS, Peter Meerwald-Stadler,
	Jonathan Cameron, Rob Herring
In-Reply-To: <20200518133358.18978-1-jonathan.albrieux@gmail.com>

Converts documentation from txt format to yaml 

Signed-off-by: Jonathan Albrieux <jonathan.albrieux@gmail.com>
---
 .../devicetree/bindings/iio/imu/bmi160.txt    | 37 --------
 .../devicetree/bindings/iio/imu/bmi160.yaml   | 84 +++++++++++++++++++
 2 files changed, 84 insertions(+), 37 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/iio/imu/bmi160.txt
 create mode 100644 Documentation/devicetree/bindings/iio/imu/bmi160.yaml

diff --git a/Documentation/devicetree/bindings/iio/imu/bmi160.txt b/Documentation/devicetree/bindings/iio/imu/bmi160.txt
deleted file mode 100644
index 900c169de00f..000000000000
--- a/Documentation/devicetree/bindings/iio/imu/bmi160.txt
+++ /dev/null
@@ -1,37 +0,0 @@
-Bosch BMI160 - Inertial Measurement Unit with Accelerometer, Gyroscope
-and externally connectable Magnetometer
-
-https://www.bosch-sensortec.com/bst/products/all_products/bmi160
-
-Required properties:
- - compatible : should be "bosch,bmi160"
- - reg : the I2C address or SPI chip select number of the sensor
- - spi-max-frequency : set maximum clock frequency (only for SPI)
-
-Optional properties:
- - interrupts : interrupt mapping for IRQ
- - interrupt-names : set to "INT1" if INT1 pin should be used as interrupt
-   input, set to "INT2" if INT2 pin should be used instead
- - drive-open-drain : set if the specified interrupt pin should be configured as
-   open drain. If not set, defaults to push-pull.
-
-Examples:
-
-bmi160@68 {
-	compatible = "bosch,bmi160";
-	reg = <0x68>;
-
-	interrupt-parent = <&gpio4>;
-	interrupts = <12 IRQ_TYPE_EDGE_RISING>;
-	interrupt-names = "INT1";
-};
-
-bmi160@0 {
-	compatible = "bosch,bmi160";
-	reg = <0>;
-	spi-max-frequency = <10000000>;
-
-	interrupt-parent = <&gpio2>;
-	interrupts = <12 IRQ_TYPE_LEVEL_LOW>;
-	interrupt-names = "INT2";
-};
diff --git a/Documentation/devicetree/bindings/iio/imu/bmi160.yaml b/Documentation/devicetree/bindings/iio/imu/bmi160.yaml
new file mode 100644
index 000000000000..6b464ce5ed0b
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/imu/bmi160.yaml
@@ -0,0 +1,84 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/imu/bmi160.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Bosch BMI160
+
+maintainers:
+  - can't find a mantainer, author is Daniel Baluta <daniel.baluta@intel.com>
+
+description: |
+  Inertial Measurement Unit with Accelerometer, Gyroscope and externally
+  connectable Magnetometer
+  https://www.bosch-sensortec.com/bst/products/all_products/bmi160
+
+properties:
+  compatible:
+    const: bosch,bmi160
+
+  reg:
+    maxItems: 1
+    description: the I2C address or SPI chip select number of the sensor
+
+  spi-max-frequency:
+    maxItems: 1
+    description: set maximum clock frequency (required only for SPI)
+
+  interrupts:
+    maxItems: 1
+    description: interrupt mapping for IRQ
+
+  interrupt-names:
+    minItems: 1
+    maxItems: 1
+    items:
+      enum:
+        - INT1
+        - INT2
+    description: |
+      set to "INT1" if INT1 pin should be used as interrupt input, set
+      to "INT2" if INT2 pin should be used instead
+
+  drive-open-drain:
+    description: |
+      set if the specified interrupt pin should be configured as
+      open drain. If not set, defaults to push-pull.
+
+required:
+  - compatible
+  - reg
+
+examples:
+  - |
+    // Example for I2C
+    i2c@78b7000 {
+        reg = <0x78b6000 0x600>;
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        bmi160@68 {
+                compatible = "bosch,bmi160";
+                reg = <0x68>;
+                interrupt-parent = <&gpio4>;
+                interrupts = <12 1>;
+                interrupt-names = "INT1";
+        };
+  - |
+    // Example for SPI
+    spi@78b7000 {
+        reg = <0x78b7000 0x600>,
+              <0x7884000 0x23000>;
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        bmi160@0 {
+                compatible = "bosch,bmi160";
+                reg = <0>;
+                spi-max-frequency = <10000000>;
+                interrupt-parent = <&gpio2>;
+                interrupts = <12 1>;
+                interrupt-names = "INT2";
+        };
+    };
-- 
2.17.1


^ permalink raw reply related

* [PATCH 0/4] iio: imu: bmi160: added regulator and mount-matrix support
From: Jonathan Albrieux @ 2020-05-18 13:33 UTC (permalink / raw)
  To: linux-kernel
  Cc: Jonathan Albrieux,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Hartmut Knaack, Lars-Peter Clausen,
	open list:IIO SUBSYSTEM AND DRIVERS, Peter Meerwald-Stadler

Convert txt format documentation to yaml.
Add documentation about vdd-supply, vddio-supply and mount-matrix.

Add vdd-supply and vddio-supply support. Without this support, vdd and vddio
should be set to always-on in device tree.

Add mount-matrix binding support. As chip could have different orientations
a mount matrix support is needed to correctly translate these differences

Jonathan Albrieux (4):
  dt-bindings: iio: imu: bmi160: convert txt format to yaml
  dt-bindings: iio: imu: bmi160: add regulators and mount-matrix
  iio: imu: bmi160: added regulator support
  iio: imu: bmi160: added mount-matrix support

 .../devicetree/bindings/iio/imu/bmi160.txt    |  37 ------
 .../devicetree/bindings/iio/imu/bmi160.yaml   | 105 ++++++++++++++++++
 drivers/iio/imu/bmi160/bmi160.h               |   3 +
 drivers/iio/imu/bmi160/bmi160_core.c          |  47 +++++++-
 4 files changed, 154 insertions(+), 38 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/iio/imu/bmi160.txt
 create mode 100644 Documentation/devicetree/bindings/iio/imu/bmi160.yaml

-- 
2.17.1


^ permalink raw reply

* [V6, 2/2] media: i2c: dw9768: Add DW9768 VCM driver
From: Dongchun Zhu @ 2020-05-18 13:27 UTC (permalink / raw)
  To: linus.walleij, bgolaszewski, mchehab, andriy.shevchenko, robh+dt,
	mark.rutland, sakari.ailus, drinkcat, tfiga, matthias.bgg,
	bingbu.cao
  Cc: srv_heupstream, linux-mediatek, linux-arm-kernel, sj.huang,
	linux-media, devicetree, louis.kuo, shengnan.wang, dongchun.zhu
In-Reply-To: <20200518132731.20855-1-dongchun.zhu@mediatek.com>

Add a V4L2 sub-device driver for DW9768 voice coil motor, providing
control to set the desired focus via IIC serial interface.

Signed-off-by: Dongchun Zhu <dongchun.zhu@mediatek.com>
---
 MAINTAINERS                |   1 +
 drivers/media/i2c/Kconfig  |  13 ++
 drivers/media/i2c/Makefile |   1 +
 drivers/media/i2c/dw9768.c | 515 +++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 530 insertions(+)
 create mode 100644 drivers/media/i2c/dw9768.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 8d72c41..c92dc99 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5157,6 +5157,7 @@ L:	linux-media@vger.kernel.org
 S:	Maintained
 T:	git git://linuxtv.org/media_tree.git
 F:	Documentation/devicetree/bindings/media/i2c/dongwoon,dw9768.yaml
+F:	drivers/media/i2c/dw9768.c
 
 DONGWOON DW9807 LENS VOICE COIL DRIVER
 M:	Sakari Ailus <sakari.ailus@linux.intel.com>
diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index 125d596..afdf994 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -1040,6 +1040,19 @@ config VIDEO_DW9714
 	  capability. This is designed for linear control of
 	  voice coil motors, controlled via I2C serial interface.
 
+config VIDEO_DW9768
+	tristate "DW9768 lens voice coil support"
+	depends on I2C && VIDEO_V4L2
+	depends on PM
+	select MEDIA_CONTROLLER
+	select VIDEO_V4L2_SUBDEV_API
+	select V4L2_FWNODE
+	help
+	  This is a driver for the DW9768 camera lens voice coil.
+	  DW9768 is a 10 bit DAC with 100mA output current sink
+	  capability. This is designed for linear control of
+	  voice coil motors, controlled via I2C serial interface.
+
 config VIDEO_DW9807_VCM
 	tristate "DW9807 lens voice coil support"
 	depends on I2C && VIDEO_V4L2 && MEDIA_CONTROLLER
diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
index 77bf7d0..4057476 100644
--- a/drivers/media/i2c/Makefile
+++ b/drivers/media/i2c/Makefile
@@ -24,6 +24,7 @@ obj-$(CONFIG_VIDEO_SAA6752HS) += saa6752hs.o
 obj-$(CONFIG_VIDEO_AD5820)  += ad5820.o
 obj-$(CONFIG_VIDEO_AK7375)  += ak7375.o
 obj-$(CONFIG_VIDEO_DW9714)  += dw9714.o
+obj-$(CONFIG_VIDEO_DW9768)  += dw9768.o
 obj-$(CONFIG_VIDEO_DW9807_VCM)  += dw9807-vcm.o
 obj-$(CONFIG_VIDEO_ADV7170) += adv7170.o
 obj-$(CONFIG_VIDEO_ADV7175) += adv7175.o
diff --git a/drivers/media/i2c/dw9768.c b/drivers/media/i2c/dw9768.c
new file mode 100644
index 0000000..a0cc9f3
--- /dev/null
+++ b/drivers/media/i2c/dw9768.c
@@ -0,0 +1,515 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (c) 2020 MediaTek Inc.
+
+#include <linux/delay.h>
+#include <linux/i2c.h>
+#include <linux/module.h>
+#include <linux/pm_runtime.h>
+#include <linux/regulator/consumer.h>
+#include <media/v4l2-async.h>
+#include <media/v4l2-ctrls.h>
+#include <media/v4l2-device.h>
+#include <media/v4l2-fwnode.h>
+#include <media/v4l2-subdev.h>
+
+#define DW9768_NAME				"dw9768"
+#define DW9768_MAX_FOCUS_POS			(1024 - 1)
+/*
+ * This sets the minimum granularity for the focus positions.
+ * A value of 1 gives maximum accuracy for a desired focus position
+ */
+#define DW9768_FOCUS_STEPS			1
+
+/*
+ * Ring control and Power control register
+ * Bit[1] RING_EN
+ * 0: Direct mode
+ * 1: AAC mode (ringing control mode)
+ * Bit[0] PD
+ * 0: Normal operation mode
+ * 1: Power down mode
+ * DW9768 requires waiting time of Topr after PD reset takes place.
+ */
+#define DW9768_RING_PD_CONTROL_REG		0x02
+#define DW9768_PD_MODE_OFF			0x00
+#define DW9768_PD_MODE_EN			BIT(0)
+#define DW9768_AAC_MODE_EN			BIT(1)
+
+/*
+ * DW9768 separates two registers to control the VCM position.
+ * One for MSB value, another is LSB value.
+ * DAC_MSB: D[9:8] (ADD: 0x03)
+ * DAC_LSB: D[7:0] (ADD: 0x04)
+ * D[9:0] DAC data input: positive output current = D[9:0] / 1023 * 100[mA]
+ */
+#define DW9768_MSB_ADDR				0x03
+#define DW9768_LSB_ADDR				0x04
+#define DW9768_STATUS_ADDR			0x05
+
+/*
+ * AAC mode control & prescale register
+ * Bit[7:5] Namely AC[2:0], decide the VCM mode and operation time.
+ * 000 Direct(default)
+ * 001 AAC2 0.48xTvib
+ * 010 AAC3 0.70xTvib
+ * 011 AAC4 0.75xTvib
+ * 100 Reserved
+ * 101 AAC8 1.13xTvib
+ * 110 Reserved
+ * 111 Reserved
+ * Bit[2:0] Namely PRESC[2:0], set the internal clock dividing rate as follow.
+ * 000 2
+ * 001 1(default)
+ * 010 1/2
+ * 011 1/4
+ * 100 8
+ * 101 4
+ * 110 Reserved
+ * 111 Reserved
+ */
+#define DW9768_AAC_PRESC_REG			0x06
+#define DW9768_AAC_MODE_SEL_MASK		GENMASK(7, 5)
+#define DW9768_CLOCK_PRE_SCALE_SEL_MASK		GENMASK(2, 0)
+
+/*
+ * VCM period of vibration register
+ * Bit[5:0] Defined as VCM rising periodic time (Tvib) together with PRESC[2:0]
+ * Tvib = (6.3ms + AACT[5:0] * 0.1ms) * Dividing Rate
+ * Dividing Rate is the internal clock dividing rate that is defined at
+ * PRESCALE register (ADD: 0x06)
+ */
+#define DW9768_AAC_TIME_REG			0x07
+
+/*
+ * DW9768 requires waiting time (delay time) of t_OPR after power-up,
+ * or in the case of PD reset taking place.
+ */
+#define DW9768_T_OPR_US				1000
+
+/*
+ * This acts as the minimum granularity of lens movement.
+ * Keep this value power of 2, so the control steps can be
+ * uniformly adjusted for gradual lens movement, with desired
+ * number of control steps.
+ */
+#define DW9768_MOVE_STEPS			16
+
+/*
+ * DW9768_AAC_PRESC_REG & DW9768_AAC_TIME_REG determine VCM operation time.
+ * If DW9768_AAC_PRESC_REG is 0x41, and DW9768_AAC_TIME_REG is 0x39, VCM mode
+ * would be AAC3, Operation Time would be 0.70xTvib, that is 8.40ms.
+ */
+#define DW9768_MOVE_DELAY_US			8400
+#define DW9768_STABLE_TIME_US			20000
+
+static const char * const dw9768_supply_names[] = {
+	"vin",	/* I2C I/O interface power */
+	"vdd",	/* VCM power */
+};
+
+/* dw9768 device structure */
+struct dw9768 {
+	struct regulator_bulk_data supplies[ARRAY_SIZE(dw9768_supply_names)];
+	struct v4l2_ctrl_handler ctrls;
+	struct v4l2_ctrl	*focus;
+	struct v4l2_subdev	sd;
+
+	u32			aac_mode;
+	u32			aac_timing;
+	u32			clock_dividing_rate;
+	bool			aac_mode_control_enable;
+	bool			aact_cnt_select_enable;
+	bool			clock_dividing_rate_select_enable;
+};
+
+static inline struct dw9768 *sd_to_dw9768(struct v4l2_subdev *subdev)
+{
+	return container_of(subdev, struct dw9768, sd);
+}
+
+struct regval_list {
+	u8 reg_num;
+	u8 value;
+};
+
+static int dw9768_read_smbus(struct dw9768 *dw9768, unsigned char reg,
+			     unsigned char *val)
+{
+	struct i2c_client *client = v4l2_get_subdevdata(&dw9768->sd);
+	int ret;
+
+	ret = i2c_smbus_read_byte_data(client, reg);
+
+	if (ret < 0)
+		return ret;
+
+	*val = (unsigned char)ret;
+
+	return 0;
+}
+
+static int dw9768_mod_reg(struct dw9768 *dw9768, u8 reg, u8 mask, u8 val)
+{
+	struct i2c_client *client = v4l2_get_subdevdata(&dw9768->sd);
+	u8 readval;
+	int ret;
+
+	ret = dw9768_read_smbus(dw9768, reg, &readval);
+	if (ret)
+		return ret;
+
+	val = (readval & ~mask) | (val & mask);
+
+	return i2c_smbus_write_byte_data(client, reg, val);
+}
+
+static int dw9768_set_dac(struct dw9768 *dw9768, u16 val)
+{
+	struct i2c_client *client = v4l2_get_subdevdata(&dw9768->sd);
+
+	/* Write VCM position to registers */
+	return i2c_smbus_write_word_swapped(client, DW9768_MSB_ADDR, val);
+}
+
+static int dw9768_init(struct dw9768 *dw9768)
+{
+	struct i2c_client *client = v4l2_get_subdevdata(&dw9768->sd);
+	int ret, val;
+
+	/* Reset DW9768_RING_PD_CONTROL_REG to default status 0x00 */
+	ret = i2c_smbus_write_byte_data(client, DW9768_RING_PD_CONTROL_REG,
+					DW9768_PD_MODE_OFF);
+	if (ret < 0)
+		return ret;
+
+	/*
+	 * DW9769 requires waiting delay time of t_OPR
+	 * after PD reset takes place.
+	 */
+	usleep_range(DW9768_T_OPR_US, DW9768_T_OPR_US + 100);
+
+	/* Set DW9768_RING_PD_CONTROL_REG to DW9768_AAC_MODE_EN(0x01) */
+	ret = i2c_smbus_write_byte_data(client, DW9768_RING_PD_CONTROL_REG,
+					DW9768_AAC_MODE_EN);
+	if (ret < 0)
+		return ret;
+
+	/* Set AAC mode according to DT property */
+	if (dw9768->aac_mode_control_enable) {
+		ret = dw9768_mod_reg(dw9768, DW9768_AAC_PRESC_REG,
+				     DW9768_AAC_MODE_SEL_MASK,
+				     dw9768->aac_mode << 5);
+		if (ret < 0)
+			return ret;
+	}
+
+	/* Set Clock Dividing Rate factor according to DT property */
+	if (dw9768->clock_dividing_rate_select_enable) {
+		ret = dw9768_mod_reg(dw9768, DW9768_AAC_PRESC_REG,
+				     DW9768_CLOCK_PRE_SCALE_SEL_MASK,
+				     dw9768->clock_dividing_rate);
+		if (ret < 0)
+			return ret;
+	}
+
+	/* Set AAC Timing according to DT property */
+	if (dw9768->aact_cnt_select_enable) {
+		ret = i2c_smbus_write_byte_data(client, DW9768_AAC_TIME_REG,
+						dw9768->aac_timing);
+		if (ret < 0)
+			return ret;
+	}
+
+	for (val = dw9768->focus->val % DW9768_MOVE_STEPS;
+	     val <= dw9768->focus->val;
+	     val += DW9768_MOVE_STEPS) {
+		ret = dw9768_set_dac(dw9768, val);
+		if (ret) {
+			dev_err(&client->dev, "%s I2C failure: %d",
+				__func__, ret);
+			return ret;
+		}
+		usleep_range(DW9768_MOVE_DELAY_US,
+			     DW9768_MOVE_DELAY_US + 1000);
+	}
+
+	return 0;
+}
+
+static int dw9768_release(struct dw9768 *dw9768)
+{
+	struct i2c_client *client = v4l2_get_subdevdata(&dw9768->sd);
+	int ret, val;
+
+	val = round_down(dw9768->focus->val, DW9768_MOVE_STEPS);
+	for ( ; val >= 0; val -= DW9768_MOVE_STEPS) {
+		ret = dw9768_set_dac(dw9768, val);
+		if (ret) {
+			dev_err(&client->dev, "I2C write fail: %d", ret);
+			return ret;
+		}
+		usleep_range(DW9768_MOVE_DELAY_US, DW9768_MOVE_DELAY_US + 1000);
+	}
+
+	/*
+	 * Wait for the motor to stabilize after the last movement
+	 * to prevent the motor from shaking.
+	 */
+	usleep_range(DW9768_STABLE_TIME_US - DW9768_MOVE_DELAY_US,
+		     DW9768_STABLE_TIME_US - DW9768_MOVE_DELAY_US + 1000);
+
+	ret = i2c_smbus_write_byte_data(client, DW9768_RING_PD_CONTROL_REG,
+					DW9768_PD_MODE_EN);
+	if (ret < 0)
+		return ret;
+
+	/*
+	 * DW9769 requires waiting delay time of t_OPR
+	 * after PD reset takes place.
+	 */
+	usleep_range(DW9768_T_OPR_US, DW9768_T_OPR_US + 100);
+
+	return 0;
+}
+
+static int dw9768_runtime_suspend(struct device *dev)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	struct v4l2_subdev *sd = i2c_get_clientdata(client);
+	struct dw9768 *dw9768 = sd_to_dw9768(sd);
+
+	dw9768_release(dw9768);
+	regulator_bulk_disable(ARRAY_SIZE(dw9768_supply_names),
+			       dw9768->supplies);
+
+	return 0;
+}
+
+static int dw9768_runtime_resume(struct device *dev)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	struct v4l2_subdev *sd = i2c_get_clientdata(client);
+	struct dw9768 *dw9768 = sd_to_dw9768(sd);
+	int ret;
+
+	ret = regulator_bulk_enable(ARRAY_SIZE(dw9768_supply_names),
+				    dw9768->supplies);
+	if (ret < 0) {
+		dev_err(dev, "failed to enable regulators\n");
+		return ret;
+	}
+
+	/*
+	 * The datasheet refers to t_OPR that needs to be waited before sending
+	 * I2C commands after power-up.
+	 */
+	usleep_range(DW9768_T_OPR_US, DW9768_T_OPR_US + 100);
+
+	ret = dw9768_init(dw9768);
+	if (ret < 0)
+		goto disable_regulator;
+
+	return 0;
+
+disable_regulator:
+	regulator_bulk_disable(ARRAY_SIZE(dw9768_supply_names),
+			       dw9768->supplies);
+
+	return ret;
+}
+
+static int dw9768_set_ctrl(struct v4l2_ctrl *ctrl)
+{
+	struct dw9768 *dw9768 = container_of(ctrl->handler,
+					     struct dw9768, ctrls);
+
+	if (ctrl->id == V4L2_CID_FOCUS_ABSOLUTE)
+		return dw9768_set_dac(dw9768, ctrl->val);
+
+	return 0;
+}
+
+static const struct v4l2_ctrl_ops dw9768_ctrl_ops = {
+	.s_ctrl = dw9768_set_ctrl,
+};
+
+static int dw9768_open(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
+{
+	int ret;
+
+	ret = pm_runtime_get_sync(sd->dev);
+	if (ret < 0) {
+		pm_runtime_put_noidle(sd->dev);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int dw9768_close(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
+{
+	pm_runtime_put(sd->dev);
+
+	return 0;
+}
+
+static const struct v4l2_subdev_internal_ops dw9768_int_ops = {
+	.open = dw9768_open,
+	.close = dw9768_close,
+};
+
+static const struct v4l2_subdev_ops dw9768_ops = { };
+
+static int dw9768_init_controls(struct dw9768 *dw9768)
+{
+	struct v4l2_ctrl_handler *hdl = &dw9768->ctrls;
+	const struct v4l2_ctrl_ops *ops = &dw9768_ctrl_ops;
+
+	v4l2_ctrl_handler_init(hdl, 1);
+
+	dw9768->focus = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_FOCUS_ABSOLUTE, 0,
+					  DW9768_MAX_FOCUS_POS,
+					  DW9768_FOCUS_STEPS, 0);
+
+	if (hdl->error)
+		return hdl->error;
+
+	dw9768->sd.ctrl_handler = hdl;
+
+	return 0;
+}
+
+static int dw9768_probe(struct i2c_client *client)
+{
+	struct device *dev = &client->dev;
+	struct dw9768 *dw9768;
+	unsigned int aac_mode_select;
+	unsigned int aac_timing_select;
+	unsigned int clock_dividing_rate_select;
+	unsigned int i;
+	int ret;
+
+	dw9768 = devm_kzalloc(dev, sizeof(*dw9768), GFP_KERNEL);
+	if (!dw9768)
+		return -ENOMEM;
+
+	v4l2_i2c_subdev_init(&dw9768->sd, client, &dw9768_ops);
+	dw9768->aac_mode_control_enable = false;
+	dw9768->aact_cnt_select_enable = false;
+	dw9768->clock_dividing_rate_select_enable = false;
+
+	/* Optional indication of AAC mode select */
+	ret = fwnode_property_read_u32(dev_fwnode(dev), "dongwoon,aac-mode",
+				       &aac_mode_select);
+
+	if (!ret) {
+		dw9768->aac_mode_control_enable = true;
+		dw9768->aac_mode = aac_mode_select;
+	}
+
+	/* Optional indication of VCM internal clock dividing rate select */
+	ret = fwnode_property_read_u32(dev_fwnode(dev),
+				       "dongwoon,clock-dividing-rate",
+				       &clock_dividing_rate_select);
+
+	if (!ret) {
+		dw9768->clock_dividing_rate_select_enable = true;
+		dw9768->clock_dividing_rate = clock_dividing_rate_select;
+	}
+
+	/* Optional indication of AAC Timing */
+	ret = fwnode_property_read_u32(dev_fwnode(dev), "dongwoon,aac-timing",
+				       &aac_timing_select);
+
+	if (!ret) {
+		dw9768->aact_cnt_select_enable = true;
+		dw9768->aac_timing = aac_timing_select;
+	}
+
+	for (i = 0; i < ARRAY_SIZE(dw9768_supply_names); i++)
+		dw9768->supplies[i].supply = dw9768_supply_names[i];
+
+	ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(dw9768_supply_names),
+				      dw9768->supplies);
+	if (ret) {
+		dev_err(dev, "failed to get regulators\n");
+		return ret;
+	}
+
+	ret = dw9768_init_controls(dw9768);
+	if (ret)
+		goto entity_cleanup;
+
+	dw9768->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
+	dw9768->sd.internal_ops = &dw9768_int_ops;
+
+	ret = media_entity_pads_init(&dw9768->sd.entity, 0, NULL);
+	if (ret < 0)
+		goto entity_cleanup;
+
+	dw9768->sd.entity.function = MEDIA_ENT_F_LENS;
+
+	pm_runtime_enable(dev);
+	if (!pm_runtime_enabled(dev)) {
+		ret = dw9768_runtime_resume(dev);
+		if (ret < 0) {
+			dev_err(dev, "failed to power on: %d\n", ret);
+			goto entity_cleanup;
+		}
+	}
+
+	ret = v4l2_async_register_subdev(&dw9768->sd);
+	if (ret < 0)
+		goto entity_cleanup;
+
+	return 0;
+
+entity_cleanup:
+	v4l2_ctrl_handler_free(&dw9768->ctrls);
+	media_entity_cleanup(&dw9768->sd.entity);
+	return ret;
+}
+
+static int dw9768_remove(struct i2c_client *client)
+{
+	struct v4l2_subdev *sd = i2c_get_clientdata(client);
+	struct dw9768 *dw9768 = sd_to_dw9768(sd);
+
+	pm_runtime_disable(&client->dev);
+	v4l2_async_unregister_subdev(&dw9768->sd);
+	v4l2_ctrl_handler_free(&dw9768->ctrls);
+	media_entity_cleanup(&dw9768->sd.entity);
+	if (!pm_runtime_status_suspended(&client->dev))
+		dw9768_runtime_suspend(&client->dev);
+	pm_runtime_set_suspended(&client->dev);
+
+	return 0;
+}
+
+static const struct of_device_id dw9768_of_table[] = {
+	{ .compatible = "dongwoon,dw9768" },
+	{ .compatible = "giantec,gt9769" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, dw9768_of_table);
+
+static const struct dev_pm_ops dw9768_pm_ops = {
+	SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
+				pm_runtime_force_resume)
+	SET_RUNTIME_PM_OPS(dw9768_runtime_suspend, dw9768_runtime_resume, NULL)
+};
+
+static struct i2c_driver dw9768_i2c_driver = {
+	.driver = {
+		.name = DW9768_NAME,
+		.pm = &dw9768_pm_ops,
+		.of_match_table = dw9768_of_table,
+	},
+	.probe_new  = dw9768_probe,
+	.remove = dw9768_remove,
+};
+module_i2c_driver(dw9768_i2c_driver);
+
+MODULE_AUTHOR("Dongchun Zhu <dongchun.zhu@mediatek.com>");
+MODULE_DESCRIPTION("DW9768 VCM driver");
+MODULE_LICENSE("GPL v2");
-- 
2.9.2

^ permalink raw reply related

* [V6, 1/2] media: dt-bindings: media: i2c: Document DW9768 bindings
From: Dongchun Zhu @ 2020-05-18 13:27 UTC (permalink / raw)
  To: linus.walleij, bgolaszewski, mchehab, andriy.shevchenko, robh+dt,
	mark.rutland, sakari.ailus, drinkcat, tfiga, matthias.bgg,
	bingbu.cao
  Cc: srv_heupstream, linux-mediatek, linux-arm-kernel, sj.huang,
	linux-media, devicetree, louis.kuo, shengnan.wang, dongchun.zhu
In-Reply-To: <20200518132731.20855-1-dongchun.zhu@mediatek.com>

Add DeviceTree binding documentation for Dongwoon Anatech DW9768 voice
coil actuator.

Signed-off-by: Dongchun Zhu <dongchun.zhu@mediatek.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
 .../bindings/media/i2c/dongwoon,dw9768.yaml        | 105 +++++++++++++++++++++
 MAINTAINERS                                        |   7 ++
 2 files changed, 112 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/i2c/dongwoon,dw9768.yaml

diff --git a/Documentation/devicetree/bindings/media/i2c/dongwoon,dw9768.yaml b/Documentation/devicetree/bindings/media/i2c/dongwoon,dw9768.yaml
new file mode 100644
index 0000000..b909e83
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/dongwoon,dw9768.yaml
@@ -0,0 +1,105 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+# Copyright (c) 2020 MediaTek Inc.
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/media/i2c/dongwoon,dw9768.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Dongwoon Anatech DW9768 Voice Coil Motor (VCM) Lens Device Tree Bindings
+
+maintainers:
+  - Dongchun Zhu <dongchun.zhu@mediatek.com>
+
+description: |-
+  The Dongwoon DW9768 is a single 10-bit digital-to-analog (DAC) converter
+  with 100 mA output current sink capability. VCM current is controlled with
+  a linear mode driver. The DAC is controlled via a 2-wire (I2C-compatible)
+  serial interface that operates at clock rates up to 1MHz. This chip
+  integrates Advanced Actuator Control (AAC) technology and is intended for
+  driving voice coil lenses in camera modules.
+
+properties:
+  compatible:
+    enum:
+      # for DW9768 VCM
+      - dongwoon,dw9768
+      # for GT9769 VCM
+      - giantec,gt9769
+
+  reg:
+    maxItems: 1
+
+  vin-supply:
+    description:
+      Definition of the regulator used as I2C I/O interface power supply.
+
+  vdd-supply:
+    description:
+      Definition of the regulator used as VCM chip power supply.
+
+  dongwoon,aac-mode:
+    description:
+      Indication of AAC mode select.
+    allOf:
+      - $ref: "/schemas/types.yaml#/definitions/uint32"
+      - enum:
+          - 0    #  Direct (default)
+          - 1    #  AAC2 (operation time# 0.48 x Tvib)
+          - 2    #  AAC3 (operation time# 0.70 x Tvib)
+          - 3    #  AAC4 (operation time# 0.75 x Tvib)
+          - 4    #  Reserved
+          - 5    #  AAC8 (operation time# 1.13 x Tvib)
+          - 6    #  Reserved
+          - 7    #  Reserved
+
+  dongwoon,aac-timing:
+    description:
+      Indication of AAC Timing count, unit of 0.1 milliseconds.
+      Valid values vary from 0 to 63 (default 32).
+    allOf:
+      - $ref: "/schemas/types.yaml#/definitions/uint32"
+
+  dongwoon,clock-dividing-rate:
+    description:
+      Indication of VCM internal clock dividing rate select, as one multiple
+      factor to calculate VCM ring periodic time Tvib.
+    allOf:
+      - $ref: "/schemas/types.yaml#/definitions/uint32"
+      - enum:
+          - 0    #  Dividing Rate -  2
+          - 1    #  Dividing Rate -  1 (default)
+          - 2    #  Dividing Rate -  1/2
+          - 3    #  Dividing Rate -  1/4
+          - 4    #  Dividing Rate -  8
+          - 5    #  Dividing Rate -  4
+          - 6    #  Dividing Rate -  Reserved
+          - 7    #  Dividing Rate -  Reserved
+
+required:
+  - compatible
+  - reg
+  - vin-supply
+  - vdd-supply
+
+additionalProperties: false
+
+examples:
+  - |
+
+    i2c {
+        clock-frequency = <400000>;
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        dw9768: camera-lens@c {
+            compatible = "dongwoon,dw9768";
+            reg = <0x0c>;
+
+            vin-supply = <&mt6358_vcamio_reg>;
+            vdd-supply = <&mt6358_vcama2_reg>;
+            dongwoon,aac-mode = <2>;
+            dongwoon,aac-timing = <57>;
+        };
+    };
+
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index e64e5db..8d72c41 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5151,6 +5151,13 @@ T:	git git://linuxtv.org/media_tree.git
 F:	Documentation/devicetree/bindings/media/i2c/dongwoon,dw9714.txt
 F:	drivers/media/i2c/dw9714.c
 
+DONGWOON DW9768 LENS VOICE COIL DRIVER
+M:	Dongchun Zhu <dongchun.zhu@mediatek.com>
+L:	linux-media@vger.kernel.org
+S:	Maintained
+T:	git git://linuxtv.org/media_tree.git
+F:	Documentation/devicetree/bindings/media/i2c/dongwoon,dw9768.yaml
+
 DONGWOON DW9807 LENS VOICE COIL DRIVER
 M:	Sakari Ailus <sakari.ailus@linux.intel.com>
 L:	linux-media@vger.kernel.org
-- 
2.9.2

^ permalink raw reply related

* [V6, 0/2] media: i2c: Add support for DW9768 VCM driver
From: Dongchun Zhu @ 2020-05-18 13:27 UTC (permalink / raw)
  To: linus.walleij, bgolaszewski, mchehab, andriy.shevchenko, robh+dt,
	mark.rutland, sakari.ailus, drinkcat, tfiga, matthias.bgg,
	bingbu.cao
  Cc: srv_heupstream, linux-mediatek, linux-arm-kernel, sj.huang,
	linux-media, devicetree, louis.kuo, shengnan.wang, dongchun.zhu

Hello,

This series adds DT bindings in YAML and V4L2 sub-device driver for DW9768
lens voice coil motor(VCM), which is a 10-bit DAC with 100mA output current
sink capability from Dongwoon.

The driver is designed for linear control of voice coil motor,
and controlled via IIC serial interface to set the desired focus.
It controls the position with 10-bit DAC data D[9:0] and seperates
two 8-bit regs to control the VCM position as belows.
DAC_MSB: D[9:8](ADDR: 0x03):
     +---+---+---+---+---+---+---+---+
     |---|---|---|---|---|---|D09|D08|
     +---+---+---+---+---+---+---+---+
DAC_LSB: D[7:0](ADDR: 0x04):
     +---+---+---+---+---+---+---+---+
     |D07|D06|D05|D04|D03|D02|D01|D00|
     +---+---+---+---+---+---+---+---+

This driver supports:
 - set DW9768 to standby mode once suspend and turn it back to active if resume
 - set the desired focus via V4L2_CID_FOCUS_ABSOLUTE ctrl

Previous versions of this patch-set can be found here:
v5: https://lore.kernel.org/linux-media/20200502161727.30463-1-dongchun.zhu@mediatek.com/
v4: https://lore.kernel.org/linux-media/20200330123634.363-1-dongchun.zhu@mediatek.com/
v3: https://lore.kernel.org/linux-media/20200228155958.20657-1-dongchun.zhu@mediatek.com/
v2: https://lore.kernel.org/linux-media/20190905072142.14606-1-dongchun.zhu@mediatek.com/
v1: https://lore.kernel.org/linux-media/20190708100641.2702-1-dongchun.zhu@mediatek.com/

Mainly changes of v6 are addressing comments from Rob, Sakari, Tomasz.
Compared to v5:
 - Add a second compatible string for the Giantec device
 - Document optional properties: "dongwoon,aac-mode", "dongwoon,aac-timing" and
   "dongwoon,clock-dividing-rate" for lens specific reg settings
 - Adjust Kconfig to match the current media tree master branch
 - Use container_of() directly to replace of defining macro function

Please help review.
Thanks.

Dongchun Zhu (2):
  media: dt-bindings: media: i2c: Document DW9768 bindings
  media: i2c: dw9768: Add DW9768 VCM driver

 .../bindings/media/i2c/dongwoon,dw9768.yaml        | 105 +++++
 MAINTAINERS                                        |   8 +
 drivers/media/i2c/Kconfig                          |  13 +
 drivers/media/i2c/Makefile                         |   1 +
 drivers/media/i2c/dw9768.c                         | 515 +++++++++++++++++++++
 5 files changed, 642 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/i2c/dongwoon,dw9768.yaml
 create mode 100644 drivers/media/i2c/dw9768.c

-- 
2.9.2

^ permalink raw reply

* Re: [PATCH v2 10/19] spi: dw: Use DMA max burst to set the request thresholds
From: Andy Shevchenko @ 2020-05-18 13:25 UTC (permalink / raw)
  To: Serge Semin
  Cc: Andy Shevchenko, Serge Semin, Mark Brown, Alexey Malahov,
	Thomas Bogendoerfer, Paul Burton, Ralf Baechle, Arnd Bergmann,
	Allison Randal, Gareth Williams, Rob Herring, linux-mips,
	devicetree, Georgy Vlasov, Ramil Zaripov, Jarkko Nikula,
	Thomas Gleixner, Wan Ahmad Zainie, Linus Walleij, Clement Leger,
	linux-spi, Linux Kernel Mailing List
In-Reply-To: <20200518125253.r4fpr4mjflclqpym@mobilestation>

On Mon, May 18, 2020 at 3:53 PM Serge Semin
<Sergey.Semin@baikalelectronics.ru> wrote:
> On Mon, May 18, 2020 at 02:03:43PM +0300, Andy Shevchenko wrote:
> > On Sat, May 16, 2020 at 11:01:33PM +0300, Serge Semin wrote:
> > > On Fri, May 15, 2020 at 05:38:42PM +0300, Andy Shevchenko wrote:
> > > > On Fri, May 15, 2020 at 01:47:49PM +0300, Serge Semin wrote:

...

> > > > >         struct dma_chan         *txchan;
> > > > > +       u32                     txburst;
> > > > >         struct dma_chan         *rxchan;
> > > > > +       u32                     rxburst;
> > > >
> > > > Leave u32 together, it may be optimal on 64-bit architectures where ABIs require padding.
> > >
> > > It's not like anyone cared about padding in this structure in the first place)
> >
> > I think I have been caring (to some extend).
>
> Well, If you have then instead of asking to rearrange just two members (which
> by the way finely grouped by the Tx-Rx affiliation) why not sending a
> patch, which would refactor the whole structure so to be optimal for the x64
> platforms? I don't really see why this gets very important for you seeing
> Mark is Ok with this. My current commit follows the common driver design
> including the DW SSI data members grouping. On the second thought I'll leave
> it as is then.

Again same issue here. What is really easy to do for you here, will
become a burden and additional churn to anybody else.
So, why not to minimize it in the first place? Same with comma in
another patch. Sorry, I really don't get it.

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply

* Re: [PATCH 17/17] ARM: dts: r8a7742: Add RWDT node
From: Lad, Prabhakar @ 2020-05-18 13:23 UTC (permalink / raw)
  To: Sergei Shtylyov
  Cc: Geert Uytterhoeven, Lad Prabhakar, Jens Axboe, Rob Herring,
	Wolfram Sang, Ulf Hansson, David S. Miller, Wim Van Sebroeck,
	Guenter Roeck, linux-ide,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Linux Kernel Mailing List, Linux I2C, Linux MMC List, netdev,
	Linux-Renesas, Linux Watchdog Mailing List
In-Reply-To: <9ab946d2-1076-ed92-0a48-9a95d798d291@cogentembedded.com>

Hi Sergei,

On Mon, May 18, 2020 at 2:17 PM Sergei Shtylyov
<sergei.shtylyov@cogentembedded.com> wrote:
>
> Hello!
>
> On 18.05.2020 15:27, Lad, Prabhakar wrote:
>
> >>> Add a device node for the Watchdog Timer (RWDT) controller on the Renesas
> >>> RZ/G1H (r8a7742) SoC.
> >>>
> >>> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >>> Reviewed-by: Marian-Cristian Rotariu <marian-cristian.rotariu.rb@bp.renesas.com>
> >>
> >> Thanks for your patch!
> >>
> >>> --- a/arch/arm/boot/dts/r8a7742.dtsi
> >>> +++ b/arch/arm/boot/dts/r8a7742.dtsi
> >>> @@ -201,6 +201,16 @@
> >>>                  #size-cells = <2>;
> >>>                  ranges;
> >>>
> >>> +               rwdt: watchdog@e6020000 {
> >>> +                       compatible = "renesas,r8a7742-wdt",
> >>> +                                    "renesas,rcar-gen2-wdt";
> >>> +                       reg = <0 0xe6020000 0 0x0c>;
> >>> +                       clocks = <&cpg CPG_MOD 402>;
> >>> +                       power-domains = <&sysc R8A7742_PD_ALWAYS_ON>;
> >>> +                       resets = <&cpg 402>;
> >>> +                       status = "disabled";
> >>
> >> Missing "interrupts" property.
> >>
> > "interrupts" property isn't used by rwdt driver  and can be dropped
> > from bindings file.
>
>     DT describes the hardware, not its driver's abilities.
>
Agreed will add, I had followed it on similar lines of r8a7743/44.

Cheers,
--Prabhakar

^ permalink raw reply

* Re: [PATCH v2 06/19] spi: dw: Discard static DW DMA slave structures
From: Andy Shevchenko @ 2020-05-18 13:22 UTC (permalink / raw)
  To: Serge Semin
  Cc: Serge Semin, Andy Shevchenko, Mark Brown, Georgy Vlasov,
	Ramil Zaripov, Alexey Malahov, Thomas Bogendoerfer, Paul Burton,
	Ralf Baechle, Allison Randal, Gareth Williams, Arnd Bergmann,
	Rob Herring, linux-mips, devicetree, Wan Ahmad Zainie,
	Jarkko Nikula, Thomas Gleixner, wuxu.wu, Clement Leger, linux-spi,
	Linux Kernel Mailing List
In-Reply-To: <20200518123242.xoosc4pcj7heo4he@mobilestation>

On Mon, May 18, 2020 at 3:32 PM Serge Semin
<Sergey.Semin@baikalelectronics.ru> wrote:
> On Mon, May 18, 2020 at 02:38:13PM +0300, Andy Shevchenko wrote:
> > On Mon, May 18, 2020 at 2:01 PM Andy Shevchenko
> > <andriy.shevchenko@linux.intel.com> wrote:
> > > On Sat, May 16, 2020 at 05:20:30PM +0300, Serge Semin wrote:

...

> > > I'm not talking about stack, it's fine for me, what I'm talking about is *how*
> > > they are being initialized. Read my message again carefully, please.
> >
> > And to avoid additional churn around this, the purpose is to show what
> > Dreq number is in use actually for these transfers (that's why 0
> > assignment is explicitly there and no counterpart Dreq filled).
>
> Sorry, but nothing persuasive so far. We still can remove their static definition
> and explicitly assign zero or one to the src and dst request IDs on stack. I'll do
> this in v3, since I have to send one anyway.

Right, I'm completely fine with it, thanks!

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply

* Re: [PATCH v7 2/2] mtd: rawnand: Add NAND controller support on Intel LGM SoC
From: Andy Shevchenko @ 2020-05-18 13:20 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Ramuthevar, Vadivel MuruganX, kbuild test robot,
	Linux Kernel Mailing List, open list:MEMORY TECHNOLOGY...,
	devicetree, kbuild-all, Miquel Raynal, Richard Weinberger,
	Vignesh R, Brendan Higgins, Thomas Gleixner, Boris Brezillon,
	Anders Roxell, masonccyang
In-Reply-To: <CAK8P3a25GbMwbtvkxgmuGss6nEfAW4_vVbOXPxOYuDOaU_zcjA@mail.gmail.com>

On Mon, May 18, 2020 at 2:57 PM Arnd Bergmann <arnd@arndb.de> wrote:
> On Mon, May 18, 2020 at 1:43 PM Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
> > On Mon, May 18, 2020 at 2:39 PM Ramuthevar, Vadivel MuruganX
> > <vadivel.muruganx.ramuthevar@linux.intel.com> wrote:
> > > On 15/5/2020 10:30 pm, Arnd Bergmann wrote:
> > > > On Fri, May 15, 2020 at 4:25 PM Andy Shevchenko
> > > > <andy.shevchenko@gmail.com> wrote:
> > > >> On Fri, May 15, 2020 at 4:48 PM kbuild test robot <lkp@intel.com> wrote:
> >
> > > > iowrite_be32() is the correct way to store word into a big-endian mmio register,
> > > > if that is the intention here.
> > > Thank you for suggestions to use iowrite32be(), it suits exactly.
> >
> > Can you before doing this comment what is the real intention here?
> >
> > And note, if you are going to use iowrite*() / ioread*() in one place,
> > you will probably need to replace all of the read*() / write*() to
> > respective io* API.
>
> The way that ioread/iowrite are defined, they are required to be a superset
> of what readl/writel do and can take __iomem pointers from either
> ioremap() or ioport_map()/pci_iomap() style mappings, while readl/writel
> are only required to work with ioremap().
>
> There is no technical requirement to stick to one set or the other for
> ioremap(), but the overhead of ioread/iowrite is also small enough
> that it generally does not hurt.

Right, my suggestion is solely for consistency. It would be a bit
weird to see readl() along with ioread32() in the same driver (in case
there are no differentiated callbacks specifically for different type
of IP).

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply

* Re: [PATCH V6 3/3] dt-bindings: serial: Add binding for UART pin swap
From: Akash Asthana @ 2020-05-18 13:19 UTC (permalink / raw)
  To: Rob Herring
  Cc: gregkh, linux-arm-msm, devicetree, linux-kernel, mgautam, rojay,
	skakit, mka
In-Reply-To: <20200515030133.GA11479@bogus>

Hi Rob,

On 5/15/2020 8:31 AM, Rob Herring wrote:
> On Thu, May 07, 2020 at 08:30:47PM +0530, Akash Asthana wrote:
>> Add documentation to support RX-TX & CTS-RTS GPIO pin swap in HW.
>>
>> Signed-off-by: Akash Asthana <akashast@codeaurora.org>
>> ---
>>   Documentation/devicetree/bindings/serial/serial.yaml | 6 ++++++
>>   1 file changed, 6 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/serial/serial.yaml b/Documentation/devicetree/bindings/serial/serial.yaml
>> index 53204d9..e657dd6 100644
>> --- a/Documentation/devicetree/bindings/serial/serial.yaml
>> +++ b/Documentation/devicetree/bindings/serial/serial.yaml
>> @@ -67,6 +67,12 @@ properties:
>>         (wired and enabled by pinmux configuration).  This depends on both the
>>         UART hardware and the board wiring.
>>   
>> +  rx-tx-swap:
>> +    description: RX and TX pins are swapped.
>> +
>> +  cts-rts-swap:
>> +    description: CTS and RTS pins are swapped.
> Need 'type: boolean' on both of these.

okay, will correct in next version

Regards,

Akash

>
>> +
>>   if:
>>     required:
>>       - uart-has-rtscts
>> -- 
>> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,\na Linux Foundation Collaborative Project

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,\na Linux Foundation Collaborative Project

^ 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