mbox series

[V9,0/2] media: i2c: Add support for DW9768 VCM

Message ID 20200630062211.22988-1-dongchun.zhu@mediatek.com (mailing list archive)
Headers show
Series media: i2c: Add support for DW9768 VCM | expand

Message

Dongchun Zhu June 30, 2020, 6:22 a.m. UTC
Hello,

This series adds DT bindings and V4L2 sub-device driver for Dongwoon's DW9768,
which is a 10-bit DAC with 100mA output current sink capability.

The driver is designed for linear control of voice coil motor(VCM).
It creates a V4L2 sub-device and provides control to set the desired focus.

It controls the position with 10-bit DAC data D[9:0] and seperates
two 8-bit registers 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:
 v8: https://lore.kernel.org/linux-media/20200616125531.31671-1-dongchun.zhu@mediatek.com/
 v7: https://lore.kernel.org/linux-media/20200605105412.18813-1-dongchun.zhu@mediatek.com/
 v6: https://lore.kernel.org/linux-media/20200518132731.20855-1-dongchun.zhu@mediatek.com/
 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 v9 are addressing comments from Tomasz.
Compared to v8:
 - Refine dw9768_cal_move_delay() to return the value in a standard unit
 - Refine err_power_off error handler section in probe()
 - Use pm_runtime_status_suspended() in remove()

Please 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        | 100 ++++
 MAINTAINERS                                        |   8 +
 drivers/media/i2c/Kconfig                          |  12 +
 drivers/media/i2c/Makefile                         |   1 +
 drivers/media/i2c/dw9768.c                         | 554 +++++++++++++++++++++
 5 files changed, 675 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/i2c/dongwoon,dw9768.yaml
 create mode 100644 drivers/media/i2c/dw9768.c

Comments

Sakari Ailus July 1, 2020, 9:16 a.m. UTC | #1
Dongchun,

On Tue, Jun 30, 2020 at 02:22:09PM +0800, Dongchun Zhu wrote:
> Hello,
> 
> This series adds DT bindings and V4L2 sub-device driver for Dongwoon's DW9768,
> which is a 10-bit DAC with 100mA output current sink capability.
> 
> The driver is designed for linear control of voice coil motor(VCM).
> It creates a V4L2 sub-device and provides control to set the desired focus.

Thanks for the update.

There seem to be trailing whitespaces at least in the first patch, and a
conflict in MAINTAINERS vs. current media tree master.

Could you address these, please?
Bingbu Cao July 2, 2020, 5:10 a.m. UTC | #2
Hi, Dongchun

I think it need rebase on linuxtv/master.
Sakari Ailus July 2, 2020, 5:34 a.m. UTC | #3
Dongchun,

Please don't send HTML e-mail to Linux kernel related mailing lists.

On Thu, Jul 02, 2020 at 03:48:56AM +0000, Dongchun Zhu (朱东春) wrote:
>  Hi Sakari,
> 
>  Sorry to bother you again, but I am so confused about the questions you raised.
>  I just synced mainline: 5.8-rc3 tarball from https://www.kernel.org/, on which I ran the git am <patch> command.
>  The patch-applying process shows no error.
>  -----------------8<-------------------
>  [mtk15013@mtkslt307 linux]$git apply --check media-i2c-Add-support-for-DW9768-VCM.patch
>  [mtk15013@mtkslt307 linux]$git am media-i2c-Add-support-for-DW9768-VCM.patch
>  Applying: media: dt-bindings: media: i2c: Document DW9768 bindings
>  Applying: media: i2c: dw9768: Add DW9768 VCM driver
>  -----------------8<-------------------
> 
>  On the other hand, I also compared dongwoon,dw9768.yaml file with other media device dt-bindings(like imx219.yaml and ov8856.yaml).
>  It seems there are no apparent differences between them.
>  Especially, the sentence '# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)' shall be common.
>  I dunno why here dongwoon,dw9768.yaml reports trailing whitespace warnings while ov8856.yaml is silent.
> 
>  For the patch failed on MAINTAINERS, I am still curious what's wrong.
>  In fact, I locally have run parse-maintainers.pl script to check MAINTAINERS file before submitting patch.
>  The result also reports no errors.
>  -----------------8<-------------------
>  [mtk15013@mtkslt307 linux]$perl scripts/parse-maintainers.pl
>  [mtk15013@mtkslt307 linux]$ls
>  -----------------8<-------------------
> 
>  As to Base64 encoding, I checked each patch file again. They are all encoded in UTF-8.
>  As https://www.base64encode.org/ says, for an example, '77' in ASCII format would be changed to 'T' in Based64-encoded format.
>  This means there shall be messy code if we adpoting Based64-encoded format.
>  But I cannot see garbled messages in the current patches.
> 
>  The DW9768 serials-patch is attached.
>  @Tomasz @Andy @Rob could anyone help try to see whether the patch can be cherry-picked on Linux master branch or not?
>  Patchwork link:
>  https://patchwork.kernel.org/cover/11633291/

Both of the patches appear to contain only ASCII characters.

I did some research on this. It seems that the base64 encoded message body
does have carriage returns, in both cases. Git am does not attempt to
remove them in that case, but Patchwork does. Hence the files are fine if
you download them from Patchwork --- where the mbox files have neither
carriage returns nor base64 encoding.

What does the file command say about the patch files produced by git
format-patch? My guess is that something in between your local computer and
LMML (and other mail servers) base64 encodes the message body. But where
are the carriage returns added? Nevertheless they seem to be added before
the base64 conversion.

I think this is also a git issue, but something that is very hard to
encounter.

...

>  ************* MEDIATEK Confidentiality Notice ********************
>  The information contained in this e-mail message (including any
>  attachments) may be confidential, proprietary, privileged, or otherwise
>  exempt from disclosure under applicable laws. It is intended to be
>  conveyed only to the designated recipient(s). Any use, dissemination,
>  distribution, printing, retaining or copying of this e-mail (including its
>  attachments) by unintended recipient(s) is strictly prohibited and may
>  be unlawful. If you are not an intended recipient of this e-mail, or believe
>  that you have received this e-mail in error, please notify the sender
>  immediately (by replying to this e-mail), delete any and all copies of
>  this e-mail (including any attachments) from your system, and do not
>  disclose the content of this e-mail to any other person. Thank you!

Did you mean this?
Dongchun Zhu July 2, 2020, 11:06 a.m. UTC | #4
Hi Sakari,

Sorry I just sent email using outlook where default format is HTML, now
I use evolution, one Linux mail client that I used to send upstream
patch previously.

On Thu, 2020-07-02 at 08:34 +0300, Sakari Ailus wrote:
> Dongchun,
> 
> Please don't send HTML e-mail to Linux kernel related mailing lists.
> 
> On Thu, Jul 02, 2020 at 03:48:56AM +0000, Dongchun Zhu (朱东春) wrote:
> >  Hi Sakari,
> > 
> >  Sorry to bother you again, but I am so confused about the questions you raised.
> >  I just synced mainline: 5.8-rc3 tarball from https://www.kernel.org/, on which I ran the git am <patch> command.
> >  The patch-applying process shows no error.
> >  -----------------8<-------------------
> >  [mtk15013@mtkslt307 linux]$git apply --check media-i2c-Add-support-for-DW9768-VCM.patch
> >  [mtk15013@mtkslt307 linux]$git am media-i2c-Add-support-for-DW9768-VCM.patch
> >  Applying: media: dt-bindings: media: i2c: Document DW9768 bindings
> >  Applying: media: i2c: dw9768: Add DW9768 VCM driver
> >  -----------------8<-------------------
> > 
> >  On the other hand, I also compared dongwoon,dw9768.yaml file with other media device dt-bindings(like imx219.yaml and ov8856.yaml).
> >  It seems there are no apparent differences between them.
> >  Especially, the sentence '# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)' shall be common.
> >  I dunno why here dongwoon,dw9768.yaml reports trailing whitespace warnings while ov8856.yaml is silent.
> > 
> >  For the patch failed on MAINTAINERS, I am still curious what's wrong.
> >  In fact, I locally have run parse-maintainers.pl script to check MAINTAINERS file before submitting patch.
> >  The result also reports no errors.
> >  -----------------8<-------------------
> >  [mtk15013@mtkslt307 linux]$perl scripts/parse-maintainers.pl
> >  [mtk15013@mtkslt307 linux]$ls
> >  -----------------8<-------------------
> > 
> >  As to Base64 encoding, I checked each patch file again. They are all encoded in UTF-8.
> >  As https://www.base64encode.org/ says, for an example, '77' in ASCII format would be changed to 'T' in Based64-encoded format.
> >  This means there shall be messy code if we adpoting Based64-encoded format.
> >  But I cannot see garbled messages in the current patches.
> > 
> >  The DW9768 serials-patch is attached.
> >  @Tomasz @Andy @Rob could anyone help try to see whether the patch can be cherry-picked on Linux master branch or not?
> >  Patchwork link:
> >  https://patchwork.kernel.org/cover/11633291/
> 
> Both of the patches appear to contain only ASCII characters.
> 
> I did some research on this. It seems that the base64 encoded message body
> does have carriage returns, in both cases. Git am does not attempt to
> remove them in that case, but Patchwork does. Hence the files are fine if
> you download them from Patchwork --- where the mbox files have neither
> carriage returns nor base64 encoding.
> 
> What does the file command say about the patch files produced by git
> format-patch? My guess is that something in between your local computer and
> LMML (and other mail servers) base64 encodes the message body. But where
> are the carriage returns added? Nevertheless they seem to be added before
> the base64 conversion.
> 

Hm... I used the file command to check the diff patch generated from git
format-patch and that downloaded from Patchwork, they are both ASCII
text.

In fact, we could also open the diff patch with notepad++ tool, if the
EOL conversion is UNIX/OSX Format, end-of-line character would be LF.
For the DW9768 patch, when we click the toolbar button "Show All
Characters", there is no carriage return(CR) at the end of each line,
but LF instead for all EOLs.

> I think this is also a git issue, but something that is very hard to
> encounter.
> 
> ...
> 
> >  ************* MEDIATEK Confidentiality Notice ********************
> >  The information contained in this e-mail message (including any
> >  attachments) may be confidential, proprietary, privileged, or otherwise
> >  exempt from disclosure under applicable laws. It is intended to be
> >  conveyed only to the designated recipient(s). Any use, dissemination,
> >  distribution, printing, retaining or copying of this e-mail (including its
> >  attachments) by unintended recipient(s) is strictly prohibited and may
> >  be unlawful. If you are not an intended recipient of this e-mail, or believe
> >  that you have received this e-mail in error, please notify the sender
> >  immediately (by replying to this e-mail), delete any and all copies of
> >  this e-mail (including any attachments) from your system, and do not
> >  disclose the content of this e-mail to any other person. Thank you!
> 
> Did you mean this?
> 

This is auto-generated by some mechanism when sending email to the
address belong to an external organization.
It mainly serves as a reminder, please don't care too much : -)
Sakari Ailus July 30, 2020, 4:39 p.m. UTC | #5
Dongchun,

On Thu, Jul 02, 2020 at 07:06:05PM +0800, Dongchun Zhu wrote:
> Hi Sakari,
> 
> Sorry I just sent email using outlook where default format is HTML, now
> I use evolution, one Linux mail client that I used to send upstream
> patch previously.
> 
> On Thu, 2020-07-02 at 08:34 +0300, Sakari Ailus wrote:
> > Dongchun,
> > 
> > Please don't send HTML e-mail to Linux kernel related mailing lists.
> > 
> > On Thu, Jul 02, 2020 at 03:48:56AM +0000, Dongchun Zhu (朱东春) wrote:
> > >  Hi Sakari,
> > > 
> > >  Sorry to bother you again, but I am so confused about the questions you raised.
> > >  I just synced mainline: 5.8-rc3 tarball from https://www.kernel.org/, on which I ran the git am <patch> command.
> > >  The patch-applying process shows no error.
> > >  -----------------8<-------------------
> > >  [mtk15013@mtkslt307 linux]$git apply --check media-i2c-Add-support-for-DW9768-VCM.patch
> > >  [mtk15013@mtkslt307 linux]$git am media-i2c-Add-support-for-DW9768-VCM.patch
> > >  Applying: media: dt-bindings: media: i2c: Document DW9768 bindings
> > >  Applying: media: i2c: dw9768: Add DW9768 VCM driver
> > >  -----------------8<-------------------
> > > 
> > >  On the other hand, I also compared dongwoon,dw9768.yaml file with other media device dt-bindings(like imx219.yaml and ov8856.yaml).
> > >  It seems there are no apparent differences between them.
> > >  Especially, the sentence '# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)' shall be common.
> > >  I dunno why here dongwoon,dw9768.yaml reports trailing whitespace warnings while ov8856.yaml is silent.
> > > 
> > >  For the patch failed on MAINTAINERS, I am still curious what's wrong.
> > >  In fact, I locally have run parse-maintainers.pl script to check MAINTAINERS file before submitting patch.
> > >  The result also reports no errors.
> > >  -----------------8<-------------------
> > >  [mtk15013@mtkslt307 linux]$perl scripts/parse-maintainers.pl
> > >  [mtk15013@mtkslt307 linux]$ls
> > >  -----------------8<-------------------
> > > 
> > >  As to Base64 encoding, I checked each patch file again. They are all encoded in UTF-8.
> > >  As https://www.base64encode.org/ says, for an example, '77' in ASCII format would be changed to 'T' in Based64-encoded format.
> > >  This means there shall be messy code if we adpoting Based64-encoded format.
> > >  But I cannot see garbled messages in the current patches.
> > > 
> > >  The DW9768 serials-patch is attached.
> > >  @Tomasz @Andy @Rob could anyone help try to see whether the patch can be cherry-picked on Linux master branch or not?
> > >  Patchwork link:
> > >  https://patchwork.kernel.org/cover/11633291/
> > 
> > Both of the patches appear to contain only ASCII characters.
> > 
> > I did some research on this. It seems that the base64 encoded message body
> > does have carriage returns, in both cases. Git am does not attempt to
> > remove them in that case, but Patchwork does. Hence the files are fine if
> > you download them from Patchwork --- where the mbox files have neither
> > carriage returns nor base64 encoding.
> > 
> > What does the file command say about the patch files produced by git
> > format-patch? My guess is that something in between your local computer and
> > LMML (and other mail servers) base64 encodes the message body. But where
> > are the carriage returns added? Nevertheless they seem to be added before
> > the base64 conversion.
> > 
> 
> Hm... I used the file command to check the diff patch generated from git
> format-patch and that downloaded from Patchwork, they are both ASCII
> text.

That's because Patchwork appears to be removing the carriage returns. git
does not if the content is base64 encoded.

Your e-mail setup simply appears to be broken. I'd suggest trying to send
the patches encoded in base64 as a workaround. git send-email uses
sendemail.transferEncoding configuration option for this.