* Re: [PATCH 12/15] arm64: dts: msm8974: thermal: Add interrupt support
From: Amit Kucheria @ 2019-07-29 9:29 UTC (permalink / raw)
To: Luca Weiss
Cc: Linux Kernel Mailing List, linux-arm-msm, Bjorn Andersson,
Eduardo Valentin, Andy Gross, Andy Gross, Daniel Lezcano,
Mark Rutland, Rob Herring, Zhang Rui, Brian Masney, DTML
In-Reply-To: <2812534.bLfc0ztHNv@g550jk>
On Mon, Jul 29, 2019 at 2:33 PM Luca Weiss <luca@z3ntu.xyz> wrote:
>
> On Freitag, 26. Juli 2019 00:18:47 CEST Amit Kucheria wrote:
> > Register upper-lower interrupt for the tsens controller.
> >
> > Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org>
> > ---
> > Cc: masneyb@onstation.org
> >
> > arch/arm/boot/dts/qcom-msm8974.dtsi | 36 +++++++++++++++--------------
> > 1 file changed, 19 insertions(+), 17 deletions(-)
> >
>
> Hi, the title of this patch should be "arm" and not "arm64".
Good catch! Copy-paste error, will fix.
^ permalink raw reply
* Re: [PATCH 00/15] thermal: qcom: tsens: Add interrupt support
From: Luca Weiss @ 2019-07-29 9:32 UTC (permalink / raw)
To: linux-kernel
Cc: Brian Masney, Amit Kucheria, linux-arm-msm, Bjorn Andersson,
Eduardo Valentin, Andy Gross, Andy Gross, Daniel Lezcano,
Mark Rutland, Rob Herring, Zhang Rui, Marc Gonzalez,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Linux PM list
In-Reply-To: <20190729090735.GA897@onstation.org>
[-- Attachment #1: Type: text/plain, Size: 1591 bytes --]
On Montag, 29. Juli 2019 11:07:35 CEST Brian Masney wrote:
> On Sat, Jul 27, 2019 at 12:58:54PM +0530, Amit Kucheria wrote:
> > On Fri, Jul 26, 2019 at 4:59 PM Brian Masney <masneyb@onstation.org> wrote:
> > > On Fri, Jul 26, 2019 at 04:40:16PM +0530, Amit Kucheria wrote:
> > > > How well does cpufreq work on 8974? I haven't looked at it yet but
> > > > we'll need it for thermal throttling.
> > >
> > > I'm not sure how to tell if the frequency is dynamically changed during
> > > runtime on arm. x86-64 shows this information in /proc/cpuinfo. Here's
> >
> > > the /proc/cpuinfo on the Nexus 5:
> > Nah. /proc/cpuinfo won't show what we need.
> >
> > Try the following:
> >
> > $ grep "" /sys/devices/system/cpu/cpufreq/policy?/*
> >
> > More specifically, the following files have the information you need.
> > Run watch -n1 on them.
> >
> > $ grep "" /sys/devices/system/cpu/cpufreq/policy?/scaling_*_freq
>
> There's no cpufreq directory on msm8974:
>
> # ls -1 /sys/devices/system/cpu/
> cpu0
> cpu1
> cpu2
> cpu3
> cpuidle
> hotplug
> isolated
> kernel_max
> modalias
> offline
> online
> possible
> power
> present
> smt
> uevent
>
> I'm using qcom_defconfig.
>
> Brian
Hi Brian,
cpufreq isn't supported on msm8974 yet.
I have these patches [0] in my tree but I'm not sure they work correctly, but I haven't tested much with them. Feel free to try them on hammerhead.
Luca
[0] https://github.com/z3ntu/linux/compare/b0917f53ada0e929896a094b451219cd8091366e...6459ca6aff498c9d12acd35709b4903effc4c3f8
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v15 2/2] dt-bindings: spi: Document Renesas R-Car Gen3 RPC-IF controller bindings
From: masonccyang @ 2019-07-29 9:34 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Mark Brown,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Geert Uytterhoeven, Simon Horman, juliensu, Lee Jones,
Linux Kernel Mailing List, Linux-Renesas, linux-spi, Marek Vasut,
Mark Rutland, Miquel Raynal, Rob Herring, Sergei Shtylyov
In-Reply-To: <CAMuHMdWVuQa1LLXPqrdSw6wdRzwQapAkk6Est=XrjESPF9zQwg@mail.gmail.com>
Hi Geert,
> On Fri, Jul 26, 2019 at 4:19 AM Mason Yang <masonccyang@mxic.com.tw>
wrote:
> > Dcument the bindings used by the Renesas R-Car Gen3 RPC-IF controller.
>
> Document
Oops, sorry !
>
> >
> > Signed-off-by: Mason Yang <masonccyang@mxic.com.tw>
> > Reviewed-by: Rob Herring <robh@kernel.org>
>
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/spi/spi-renesas-rpc.txt
> > @@ -0,0 +1,46 @@
>
> [...]
>
> > +- flash: should be represented by a subnode of the RPC-IF node,
> > + which "compatible" property contains "jedec,spi-nor", it
presents
> > + SPI is used.
>
> Sorry, I failed to parse the last subsentence.
Fix it to:
- flash: should be represented by a subnode of the RPC-IF node,
its "compatible" property contains "jedec,spi-nor" presents
SPI is used.
OK?
>
> > +
> > +Example:
> > +
> > + rpc: spi@ee200000 {
> > + compatible = "renesas,r8a77995-rpc",
"renesas,rcar-gen3-rpc";
> > + reg = <0 0xee200000 0 0x200>, <0 0x08000000 0
0x4000000>,
> > + <0 0xee208000 0 0x100>;
> > + reg-names = "regs", "dirmap", "wbuf";
> > + clocks = <&cpg CPG_MOD 917>;
> > + clock-names = "rpc";
> > + power-domains = <&sysc R8A77995_PD_ALWAYS_ON>;
> > + resets = <&cpg 917>;
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + flash@0 {
> > + compatible = "jedec,spi-nor";
> > + reg = <0>;
> > + spi-max-frequency = <40000000>;
> > + spi-tx-bus-width = <1>;
> > + spi-rx-bus-width = <1>;
>
> Shouldn't those <1> be <4>, as this is QSPI?
okay, fix them to <4>
thanks for your time & review.
best regards,
Mason
CONFIDENTIALITY NOTE:
This e-mail and any attachments may contain confidential information
and/or personal data, which is protected by applicable laws. Please be
reminded that duplication, disclosure, distribution, or use of this e-mail
(and/or its attachments) or any part thereof is prohibited. If you receive
this e-mail in error, please notify us immediately and delete this mail as
well as its attachment(s) from your system. In addition, please be
informed that collection, processing, and/or use of personal data is
prohibited unless expressly permitted by personal data protection laws.
Thank you for your attention and cooperation.
Macronix International Co., Ltd.
=====================================================================
============================================================================
CONFIDENTIALITY NOTE:
This e-mail and any attachments may contain confidential information and/or personal data, which is protected by applicable laws. Please be reminded that duplication, disclosure, distribution, or use of this e-mail (and/or its attachments) or any part thereof is prohibited. If you receive this e-mail in error, please notify us immediately and delete this mail as well as its attachment(s) from your system. In addition, please be informed that collection, processing, and/or use of personal data is prohibited unless expressly permitted by personal data protection laws. Thank you for your attention and cooperation.
Macronix International Co., Ltd.
=====================================================================
^ permalink raw reply
* Re: [PATCH v4 0/3] Introduce Bandwidth OPPs for interconnects
From: Viresh Kumar @ 2019-07-29 9:35 UTC (permalink / raw)
To: Saravana Kannan
Cc: Rob Herring, Mark Rutland, Viresh Kumar, Nishanth Menon,
Stephen Boyd, Rafael J. Wysocki, Georgi Djakov, vincent.guittot,
seansw, daidavid1, adharmap, Rajendra Nayak, sibis,
bjorn.andersson, evgreen, kernel-team, linux-pm, devicetree,
linux-kernel
In-Reply-To: <20190726231558.175130-1-saravanak@google.com>
On 26-07-19, 16:15, Saravana Kannan wrote:
> Interconnects and interconnect paths quantify their performance levels in
> terms of bandwidth and not in terms of frequency. So similar to how we have
> frequency based OPP tables in DT and in the OPP framework, we need
> bandwidth OPP table support in DT and in the OPP framework.
>
> So with the DT bindings added in this patch series, the DT for a GPU
> that does bandwidth voting from GPU to Cache and GPU to DDR would look
> something like this:
>
> gpu_cache_opp_table: gpu_cache_opp_table {
> compatible = "operating-points-v2";
>
> gpu_cache_3000: opp-3000 {
> opp-peak-KBps = <3000000>;
> opp-avg-KBps = <1000000>;
> };
> gpu_cache_6000: opp-6000 {
> opp-peak-KBps = <6000000>;
> opp-avg-KBps = <2000000>;
> };
> gpu_cache_9000: opp-9000 {
> opp-peak-KBps = <9000000>;
> opp-avg-KBps = <9000000>;
> };
> };
>
> gpu_ddr_opp_table: gpu_ddr_opp_table {
> compatible = "operating-points-v2";
>
> gpu_ddr_1525: opp-1525 {
> opp-peak-KBps = <1525000>;
> opp-avg-KBps = <452000>;
> };
> gpu_ddr_3051: opp-3051 {
> opp-peak-KBps = <3051000>;
> opp-avg-KBps = <915000>;
> };
> gpu_ddr_7500: opp-7500 {
> opp-peak-KBps = <7500000>;
> opp-avg-KBps = <3000000>;
> };
> };
>
> gpu_opp_table: gpu_opp_table {
> compatible = "operating-points-v2";
> opp-shared;
>
> opp-200000000 {
> opp-hz = /bits/ 64 <200000000>;
> };
> opp-400000000 {
> opp-hz = /bits/ 64 <400000000>;
> };
> };
>
> gpu@7864000 {
> ...
> operating-points-v2 = <&gpu_opp_table>, <&gpu_cache_opp_table>, <&gpu_ddr_opp_table>;
> ...
> };
One feedback I missed giving earlier. Will it be possible to get some
user code merged along with this ? I want to make sure anything we add
ends up getting used.
That also helps understanding the problems you are facing in a better
way, i.e. with real examples.
--
viresh
^ permalink raw reply
* Re: [PATCH v15 2/2] dt-bindings: spi: Document Renesas R-Car Gen3 RPC-IF controller bindings
From: Geert Uytterhoeven @ 2019-07-29 9:36 UTC (permalink / raw)
To: Mason Yang
Cc: Mark Brown,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Geert Uytterhoeven, Simon Horman, juliensu, Lee Jones,
Linux Kernel Mailing List, Linux-Renesas, linux-spi, Marek Vasut,
Mark Rutland, Miquel Raynal, Rob Herring, Sergei Shtylyov
In-Reply-To: <OFFF9D8385.D8395BBD-ON48258446.0033FD62-48258446.00349608@mxic.com.tw>
Hi Mason,
On Mon, Jul 29, 2019 at 11:34 AM <masonccyang@mxic.com.tw> wrote:
> > On Fri, Jul 26, 2019 at 4:19 AM Mason Yang <masonccyang@mxic.com.tw>
> wrote:
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/spi/spi-renesas-rpc.txt
> > > @@ -0,0 +1,46 @@
> >
> > [...]
> >
> > > +- flash: should be represented by a subnode of the RPC-IF node,
> > > + which "compatible" property contains "jedec,spi-nor", it
> presents
> > > + SPI is used.
> >
> > Sorry, I failed to parse the last subsentence.
>
>
> Fix it to:
> - flash: should be represented by a subnode of the RPC-IF node,
> its "compatible" property contains "jedec,spi-nor" presents
> SPI is used.
>
> OK?
s/presents/if/?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH v15 2/2] dt-bindings: spi: Document Renesas R-Car Gen3 RPC-IF controller bindings
From: masonccyang @ 2019-07-29 9:44 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Mark Brown,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Geert Uytterhoeven, Simon Horman, juliensu, Lee Jones,
Linux Kernel Mailing List, Linux-Renesas, linux-spi, Marek Vasut,
Mark Rutland, Miquel Raynal, Rob Herring, Sergei Shtylyov
In-Reply-To: <CAMuHMdXDJFWHaXk3KHQqhOqVhEGc3PL33f+HO2Ld8nYbvSpq7Q@mail.gmail.com>
Hi Geert,
> On Mon, Jul 29, 2019 at 11:34 AM <masonccyang@mxic.com.tw> wrote:
> > > On Fri, Jul 26, 2019 at 4:19 AM Mason Yang <masonccyang@mxic.com.tw>
> > wrote:
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/spi/spi-renesas-rpc.txt
> > > > @@ -0,0 +1,46 @@
> > >
> > > [...]
> > >
> > > > +- flash: should be represented by a subnode of the RPC-IF node,
> > > > + which "compatible" property contains "jedec,spi-nor", it
> > presents
> > > > + SPI is used.
> > >
> > > Sorry, I failed to parse the last subsentence.
> >
> >
> > Fix it to:
> > - flash: should be represented by a subnode of the RPC-IF node,
> > its "compatible" property contains "jedec,spi-nor" presents
> > SPI is used.
> >
> > OK?
>
> s/presents/if/?
>
Got it, thanks for your correction.
- flash: should be represented by a subnode of the RPC-IF node,
its "compatible" property contains "jedec,spi-nor" if SPI is
used.
best regards,
Mason
CONFIDENTIALITY NOTE:
This e-mail and any attachments may contain confidential information
and/or personal data, which is protected by applicable laws. Please be
reminded that duplication, disclosure, distribution, or use of this e-mail
(and/or its attachments) or any part thereof is prohibited. If you receive
this e-mail in error, please notify us immediately and delete this mail as
well as its attachment(s) from your system. In addition, please be
informed that collection, processing, and/or use of personal data is
prohibited unless expressly permitted by personal data protection laws.
Thank you for your attention and cooperation.
Macronix International Co., Ltd.
=====================================================================
============================================================================
CONFIDENTIALITY NOTE:
This e-mail and any attachments may contain confidential information and/or personal data, which is protected by applicable laws. Please be reminded that duplication, disclosure, distribution, or use of this e-mail (and/or its attachments) or any part thereof is prohibited. If you receive this e-mail in error, please notify us immediately and delete this mail as well as its attachment(s) from your system. In addition, please be informed that collection, processing, and/or use of personal data is prohibited unless expressly permitted by personal data protection laws. Thank you for your attention and cooperation.
Macronix International Co., Ltd.
=====================================================================
^ permalink raw reply
* Re: [PATCH 00/15] thermal: qcom: tsens: Add interrupt support
From: Amit Kucheria @ 2019-07-29 9:50 UTC (permalink / raw)
To: Luca Weiss, Niklas Cassel
Cc: LKML, Brian Masney, linux-arm-msm, Bjorn Andersson,
Eduardo Valentin, Andy Gross, Andy Gross, Daniel Lezcano,
Mark Rutland, Rob Herring, Zhang Rui, Marc Gonzalez,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Linux PM list
In-Reply-To: <2123341.TWUfUUIiFt@g550jk>
On Mon, Jul 29, 2019 at 3:03 PM Luca Weiss <luca@z3ntu.xyz> wrote:
>
> On Montag, 29. Juli 2019 11:07:35 CEST Brian Masney wrote:
> > On Sat, Jul 27, 2019 at 12:58:54PM +0530, Amit Kucheria wrote:
> > > On Fri, Jul 26, 2019 at 4:59 PM Brian Masney <masneyb@onstation.org> wrote:
> > > > On Fri, Jul 26, 2019 at 04:40:16PM +0530, Amit Kucheria wrote:
> > > > > How well does cpufreq work on 8974? I haven't looked at it yet but
> > > > > we'll need it for thermal throttling.
> > > >
> > > > I'm not sure how to tell if the frequency is dynamically changed during
> > > > runtime on arm. x86-64 shows this information in /proc/cpuinfo. Here's
> > >
> > > > the /proc/cpuinfo on the Nexus 5:
> > > Nah. /proc/cpuinfo won't show what we need.
> > >
> > > Try the following:
> > >
> > > $ grep "" /sys/devices/system/cpu/cpufreq/policy?/*
> > >
> > > More specifically, the following files have the information you need.
> > > Run watch -n1 on them.
> > >
> > > $ grep "" /sys/devices/system/cpu/cpufreq/policy?/scaling_*_freq
> >
> > There's no cpufreq directory on msm8974:
> >
> > # ls -1 /sys/devices/system/cpu/
> > cpu0
> > cpu1
> > cpu2
> > cpu3
> > cpuidle
> > hotplug
> > isolated
> > kernel_max
> > modalias
> > offline
> > online
> > possible
> > power
> > present
> > smt
> > uevent
> >
> > I'm using qcom_defconfig.
> >
> > Brian
>
> Hi Brian,
> cpufreq isn't supported on msm8974 yet.
> I have these patches [0] in my tree but I'm not sure they work correctly, but I haven't tested much with them. Feel free to try them on hammerhead.
>
> Luca
>
> [0] https://github.com/z3ntu/linux/compare/b0917f53ada0e929896a094b451219cd8091366e...6459ca6aff498c9d12acd35709b4903effc4c3f8
Niklas is working on refactoring some of the Krait code[1]. I'm not
sure if he looked at 8974 directly as part of the refactor adding him
here to get a better sense of the state of cpufreq on 8974.
[1] https://lore.kernel.org/linux-arm-msm/20190726080823.xwhxagv5iuhudmic@vireshk-i7/T/#t
^ permalink raw reply
* Re: [RFC PATCH V2 4/4] platform: mtk-isp: Add Mediatek FD driver
From: Tomasz Figa @ 2019-07-29 9:57 UTC (permalink / raw)
To: Jerry-ch Chen, Hans Verkuil, Sakari Ailus
Cc: devicetree@vger.kernel.org, Sean Cheng (鄭昇弘),
Frederic Chen (陳俊元),
Rynn Wu (吳育恩),
Christie Yu (游雅惠), srv_heupstream,
Po-Yang Huang (黃柏陽), suleiman@chromium.org,
Sj Huang (黃信璋),
Jungo Lin (林明俊), shik@chromium.org,
yuzhao@chromium.org, Enrico Weigelt, metux IT consult,
zwisler@chromium.org, hans.verkuil@cisco.com
In-Reply-To: <1564380061.15267.383.camel@mtksdccf07>
On Mon, Jul 29, 2019 at 3:01 PM Jerry-ch Chen
<Jerry-ch.Chen@mediatek.com> wrote:
>
> Hi Enrico,
>
> On Tue, 2019-07-09 at 18:56 +0800, Enrico Weigelt, metux IT consult
> wrote:
> > On 09.07.19 10:41, Jerry-ch Chen wrote:
> >
> > Hi,
> >
> >
> > > diff --git a/drivers/media/platform/mtk-isp/fd/mtk_fd.h b/drivers/media/platform/mtk-isp/fd/mtk_fd.h
> > > new file mode 100644
> > > index 0000000..289999b
> > > --- /dev/null
> > > +++ b/drivers/media/platform/mtk-isp/fd/mtk_fd.h
> > > @@ -0,0 +1,157 @@
> > > +/* SPDX-License-Identifier: GPL-2.0 */
> > > +//
> > > +// Copyright (c) 2018 MediaTek Inc.
> > > +
> > > +#ifndef __MTK_FD_HW_H__
> > > +#define __MTK_FD_HW_H__
> > > +
> > > +#include <linux/io.h>
> > > +#include <linux/types.h>
> > > +#include <linux/platform_device.h>
> > > +#include <media/v4l2-ctrls.h>
> > > +#include <media/v4l2-device.h>
> > > +#include <media/videobuf2-v4l2.h>
> > > +
> > > +#define MTK_FD_OUTPUT_MIN_WIDTH 26U
> > > +#define MTK_FD_OUTPUT_MIN_HEIGHT 26U
> > > +#define MTK_FD_OUTPUT_MAX_WIDTH 640U
> > > +#define MTK_FD_OUTPUT_MAX_HEIGHT 480U
> > > +
> > > +/* Control the user defined image widths and heights
> > > + * to be scaled and performed face detection in FD HW.
> > > + * MTK FD support up to 14 user defined image sizes to perform face detection.
> > > + */
> > > +#define V4L2_CID_MTK_FD_SCALE_IMG_WIDTH (V4L2_CID_USER_MTK_FD_BASE + 1)
> > > +#define V4L2_CID_MTK_FD_SCALE_IMG_HEIGHT (V4L2_CID_USER_MTK_FD_BASE + 2)
> >
> > I've got a *really* bad feeling about introducing chip specific
> > uapi stuff. (by the way: uapi stuff belongs into include/uapi/...)
> >
> Thanks for your comments,
>
> If we remain chip-specific control IDs, I will move the uapi stuff into
> inlcude/uapi/mtk_fd.h (filename TBD)
>
> > Maybe you could tell us what that's *really* about, so we can find some
> > standard / chip-independent api for these things. That's one of the
> > major point of the kernel: hardware abstraction.
> >
> I am not sure if it is possible for us to add some standard
> v4l2-controls for face detection, a further explanations of controls are
> listed below.
>
> In v4l2-controls, there exists V4L2_CID_DETECT_CLASS, but I haven't
> found the standards or api that can be used for face detection yet.
> https://elixir.bootlin.com/linux/latest/source/include/uapi/linux/v4l2-controls.h#L1092
>
> For detecting certain face angle and head direction, we would need
> V4L2_CID_DETECT_ANGLE, V4L2_CID_DETECT_DIRECTION controls for user to
> specify the angle and direction to be detected.
> In MTK FD driver, we support the following angles and directions to be
> selected by user, and they are both multiple selected .
> FD_angle_table[] = {-90, -45, 0 , 45, 90}
> FD_direction_table[] = {0, 30, 60, 90, 120, 150, ..., 330}
>
> Assuming these v4l2-controls are array of V4L2_CTRL_TYPE_U16 with
> dimension 5 and 12.
> User can select the desired angle and directions to be detected into
> arrays and bring it to driver by these controls, however, the more they
> select, the longer execution time needed by HW.
>
Sounds like we need some kind of a menu bitmask control here, but I
don't see V4L2 having anything like that.
Hans, Sakari, any ideas?
> For detecting different sizes of faces and increase the detection speed,
> FD driver might need to scales down the input image into different
> smaller sizes
Do you mean the FD hardware would do the scaling or the driver code
itself? It would be undesirable to do such scaling in a kernel driver,
so if that's not something handled by the hardware, the downscaled
image might need to be provided from the userspace.
>, besides driver default values, user or proprietary
> algorithm library can manually set the desired image sizes, therefore,
> we would need the following controls:
> V4L2_CID_DETECT_SCALE_DOWN_IMG_WIDTH and
> V4L2_CID_DETECT_SCALE_DOWN_IMG_HEIGHT.
> In MTK FD driver, we implement these controls as array of
> V4L2_CTRL_TYPE_U16 with the dimension 15.
Why 15?
>
> For controlling detection speed, we would need the
> V4L2_CID_DETECT_SPEED, the faster speedup implies the lower accuracy of
> detection, In MTK FD driver, the max level of speedup is 7, and default
> value is 0.
>
> For MTK FD algorithm user library, they would need select extra
> detection features(models) used in HW, we need
> V4L2_CID_MTK_FD_EXTRA_MODEL, this will be set to 1 for using extra
> model. However, we are considering make this control more
> chip-independent and can be added into standard.
> for example, V4L2_CID_DETECTION_FD_MODEL or ...FD_ALGO,
> drivers can define the detection algorithm or detection model to be used
> for users to select. How do you think?
Sounds like something that could be a menu control, so it could vary
between drivers.
>
> In short, I summery the control IDs as following:
> V4L2_CID_DETECT_ANGLE: set the angle of face in degrees. 90 ~ -90
> degrees.
> V4L2_CID_DETECT_DIRECTION: set the rotation of the head in degrees.
> 0~330 degrees.
> V4L2_CID_DETECT_SCALE_DOWN_IMG_WIDTH: set the image widths for an input
> image to be scaled down for face detection
> V4L2_CID_DETECT_SCALE_DOWN_IMG_HEIGHT: set the image heights for an
> input image to be scaled down for face detection
> V4L2_CID_DETECT_SPEED: set the detection speed, usually reducing
> accuracy.
> V4L2_CID_DETECTION_FD_MODEL: select the detection model or algorithm to
> be used by face detection driver.
>
> > > +#define ENABLE_FD 0x111
> > > +#define FD_HW_ENABLE 0x4
> > > +#define FD_INT_EN 0x15c
> > > +#define FD_INT 0x168
> > > +#define FD_RESULT 0x178
> > > +#define FD_IRQ_MASK 0x001
> > > +
> > > +#define RS_MAX_BUF_SIZE 2288788
> > > +#define FD_MAX_SPEEDUP 7
> > > +#define FD_MAX_POSE_VAL 0xfffffffffffffff
> > > +#define FD_DEF_POSE_VAL 0x3ff
> > > +#define MAX_FD_SEL_NUM 1026
> >
> > If that file is supposed to be included by anything beyond the driver
> > itself, we need proper prefixing. (same for anything else in here)
> >
> I will fix it as following:
>
> #define FD_ENABLE 0x111
>
> #define FD_REG_OFFSET_HW_ENABLE 0x4
> #define FD_REG_OFFSET_INT_EN 0x15c
> #define FD_REG_OFFSET_INT_VAL 0x168
> #define FD_REG_OFFSET_RESULT 0x178
>
> #define FD_IRQ_MASK 1
> #define FD_MAX_RS_BUF_SIZE 2288788
> #define FD_MAX_SPEEDUP 7
> #define FD_MAX_RESULT_NUM 1026
>
I'd suggest the MTK_FD_ prefix.
> > > diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
> > > index 3dcfc61..eae876e 100644
> > > --- a/include/uapi/linux/v4l2-controls.h
> > > +++ b/include/uapi/linux/v4l2-controls.h
> > > @@ -192,6 +192,10 @@ enum v4l2_colorfx {
> > > * We reserve 16 controls for this driver. */
> > > #define V4L2_CID_USER_IMX_BASE (V4L2_CID_USER_BASE + 0x10b0)
> > >
> > > +/* The base for the mediatek FD driver controls */
> > > +/* We reserve 16 controls for this driver. */
> > > +#define V4L2_CID_USER_MTK_FD_BASE (V4L2_CID_USER_BASE + 0x10d0)
> >
> > Why only the base, but not the actual IDs in uapi ?
> >
> I will put actual IDs in uapi/ for user to reference.
>
> >
> > --mtx
> >
>
Best regards,
Tomasz
^ permalink raw reply
* [PATCH net-next v3 0/4] enetc: Add mdio bus driver for the PCIe MDIO endpoint
From: Claudiu Manoil @ 2019-07-29 10:03 UTC (permalink / raw)
To: David S . Miller
Cc: andrew, Rob Herring, Li Yang, alexandru.marginean, netdev,
devicetree, linux-arm-kernel, linux-kernel
First patch fixes a sparse issue and cleans up accessors to avoid
casting to __iomem.
Second patch just registers the PCIe endpoint device containing
the MDIO registers as a standalone MDIO bus driver, to allow
an alternative way to control the MDIO bus. The same code used
by the ENETC ports (eth controllers) to manage MDIO via local
registers applies and is reused.
Bindings are provided for the new MDIO node, similarly to ENETC
port nodes bindings.
Last patch enables the ENETC port 1 and its RGMII PHY on the
LS1028A QDS board, where the MDIO muxing configuration relies
on the MDIO support provided in the first patch.
Changes since v0:
v1 - fixed mdio bus allocation
v2 - cleaned up accessors to avoid casting
v3 - fixed spelling (mostly commit message)
Claudiu Manoil (4):
enetc: Clean up local mdio bus allocation
enetc: Add mdio bus driver for the PCIe MDIO endpoint
dt-bindings: net: fsl: enetc: Add bindings for the central MDIO PCIe
endpoint
arm64: dts: fsl: ls1028a: Enable eth port1 on the ls1028a QDS board
.../devicetree/bindings/net/fsl-enetc.txt | 42 +++-
.../boot/dts/freescale/fsl-ls1028a-qds.dts | 40 ++++
.../arm64/boot/dts/freescale/fsl-ls1028a.dtsi | 6 +
.../net/ethernet/freescale/enetc/enetc_mdio.c | 190 +++++++++++++-----
.../net/ethernet/freescale/enetc/enetc_pf.c | 5 +-
5 files changed, 232 insertions(+), 51 deletions(-)
--
2.17.1
^ permalink raw reply
* [PATCH net-next v3 1/4] enetc: Clean up local mdio bus allocation
From: Claudiu Manoil @ 2019-07-29 10:03 UTC (permalink / raw)
To: David S . Miller
Cc: andrew, Rob Herring, Li Yang, alexandru.marginean, netdev,
devicetree, linux-arm-kernel, linux-kernel
In-Reply-To: <1564394627-3810-1-git-send-email-claudiu.manoil@nxp.com>
What's needed is basically a pointer to the mdio registers.
This is one way to store it inside bus->priv allocated space,
without upsetting sparse.
Reworked accessors to avoid __iomem casting.
Used devm_* variant to further clean up the init error /
remove paths.
Fixes following sparse warning:
warning: incorrect type in assignment (different address spaces)
expected void *priv
got struct enetc_mdio_regs [noderef] <asn:2>*[assigned] regs
Fixes: ebfcb23d62ab ("enetc: Add ENETC PF level external MDIO support")
Signed-off-by: Claudiu Manoil <claudiu.manoil@nxp.com>
---
v1 - added this patch
v2 - reworked accessors as per Andrew Lunn's request
v3 - cleaned up commit message
.../net/ethernet/freescale/enetc/enetc_mdio.c | 94 +++++++++----------
1 file changed, 46 insertions(+), 48 deletions(-)
diff --git a/drivers/net/ethernet/freescale/enetc/enetc_mdio.c b/drivers/net/ethernet/freescale/enetc/enetc_mdio.c
index 77b9cd10ba2b..05094601ece8 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc_mdio.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc_mdio.c
@@ -8,16 +8,22 @@
#include "enetc_pf.h"
-struct enetc_mdio_regs {
- u32 mdio_cfg; /* MDIO configuration and status */
- u32 mdio_ctl; /* MDIO control */
- u32 mdio_data; /* MDIO data */
- u32 mdio_addr; /* MDIO address */
+#define ENETC_MDIO_REG_OFFSET 0x1c00
+#define ENETC_MDIO_CFG 0x0 /* MDIO configuration and status */
+#define ENETC_MDIO_CTL 0x4 /* MDIO control */
+#define ENETC_MDIO_DATA 0x8 /* MDIO data */
+#define ENETC_MDIO_ADDR 0xc /* MDIO address */
+
+#define enetc_mdio_rd(hw, off) \
+ enetc_port_rd(hw, ENETC_##off + ENETC_MDIO_REG_OFFSET)
+#define enetc_mdio_wr(hw, off, val) \
+ enetc_port_wr(hw, ENETC_##off + ENETC_MDIO_REG_OFFSET, val)
+#define enetc_mdio_rd_reg(off) enetc_mdio_rd(hw, off)
+
+struct enetc_mdio_priv {
+ struct enetc_hw *hw;
};
-#define bus_to_enetc_regs(bus) (struct enetc_mdio_regs __iomem *)((bus)->priv)
-
-#define ENETC_MDIO_REG_OFFSET 0x1c00
#define ENETC_MDC_DIV 258
#define MDIO_CFG_CLKDIV(x) ((((x) >> 1) & 0xff) << 8)
@@ -33,18 +39,19 @@ struct enetc_mdio_regs {
#define MDIO_DATA(x) ((x) & 0xffff)
#define TIMEOUT 1000
-static int enetc_mdio_wait_complete(struct enetc_mdio_regs __iomem *regs)
+static int enetc_mdio_wait_complete(struct enetc_hw *hw)
{
u32 val;
- return readx_poll_timeout(enetc_rd_reg, ®s->mdio_cfg, val,
+ return readx_poll_timeout(enetc_mdio_rd_reg, MDIO_CFG, val,
!(val & MDIO_CFG_BSY), 10, 10 * TIMEOUT);
}
static int enetc_mdio_write(struct mii_bus *bus, int phy_id, int regnum,
u16 value)
{
- struct enetc_mdio_regs __iomem *regs = bus_to_enetc_regs(bus);
+ struct enetc_mdio_priv *mdio_priv = bus->priv;
+ struct enetc_hw *hw = mdio_priv->hw;
u32 mdio_ctl, mdio_cfg;
u16 dev_addr;
int ret;
@@ -59,29 +66,29 @@ static int enetc_mdio_write(struct mii_bus *bus, int phy_id, int regnum,
mdio_cfg &= ~MDIO_CFG_ENC45;
}
- enetc_wr_reg(®s->mdio_cfg, mdio_cfg);
+ enetc_mdio_wr(hw, MDIO_CFG, mdio_cfg);
- ret = enetc_mdio_wait_complete(regs);
+ ret = enetc_mdio_wait_complete(hw);
if (ret)
return ret;
/* set port and dev addr */
mdio_ctl = MDIO_CTL_PORT_ADDR(phy_id) | MDIO_CTL_DEV_ADDR(dev_addr);
- enetc_wr_reg(®s->mdio_ctl, mdio_ctl);
+ enetc_mdio_wr(hw, MDIO_CTL, mdio_ctl);
/* set the register address */
if (regnum & MII_ADDR_C45) {
- enetc_wr_reg(®s->mdio_addr, regnum & 0xffff);
+ enetc_mdio_wr(hw, MDIO_ADDR, regnum & 0xffff);
- ret = enetc_mdio_wait_complete(regs);
+ ret = enetc_mdio_wait_complete(hw);
if (ret)
return ret;
}
/* write the value */
- enetc_wr_reg(®s->mdio_data, MDIO_DATA(value));
+ enetc_mdio_wr(hw, MDIO_DATA, MDIO_DATA(value));
- ret = enetc_mdio_wait_complete(regs);
+ ret = enetc_mdio_wait_complete(hw);
if (ret)
return ret;
@@ -90,7 +97,8 @@ static int enetc_mdio_write(struct mii_bus *bus, int phy_id, int regnum,
static int enetc_mdio_read(struct mii_bus *bus, int phy_id, int regnum)
{
- struct enetc_mdio_regs __iomem *regs = bus_to_enetc_regs(bus);
+ struct enetc_mdio_priv *mdio_priv = bus->priv;
+ struct enetc_hw *hw = mdio_priv->hw;
u32 mdio_ctl, mdio_cfg;
u16 dev_addr, value;
int ret;
@@ -104,41 +112,41 @@ static int enetc_mdio_read(struct mii_bus *bus, int phy_id, int regnum)
mdio_cfg &= ~MDIO_CFG_ENC45;
}
- enetc_wr_reg(®s->mdio_cfg, mdio_cfg);
+ enetc_mdio_wr(hw, MDIO_CFG, mdio_cfg);
- ret = enetc_mdio_wait_complete(regs);
+ ret = enetc_mdio_wait_complete(hw);
if (ret)
return ret;
/* set port and device addr */
mdio_ctl = MDIO_CTL_PORT_ADDR(phy_id) | MDIO_CTL_DEV_ADDR(dev_addr);
- enetc_wr_reg(®s->mdio_ctl, mdio_ctl);
+ enetc_mdio_wr(hw, MDIO_CTL, mdio_ctl);
/* set the register address */
if (regnum & MII_ADDR_C45) {
- enetc_wr_reg(®s->mdio_addr, regnum & 0xffff);
+ enetc_mdio_wr(hw, MDIO_ADDR, regnum & 0xffff);
- ret = enetc_mdio_wait_complete(regs);
+ ret = enetc_mdio_wait_complete(hw);
if (ret)
return ret;
}
/* initiate the read */
- enetc_wr_reg(®s->mdio_ctl, mdio_ctl | MDIO_CTL_READ);
+ enetc_mdio_wr(hw, MDIO_CTL, mdio_ctl | MDIO_CTL_READ);
- ret = enetc_mdio_wait_complete(regs);
+ ret = enetc_mdio_wait_complete(hw);
if (ret)
return ret;
/* return all Fs if nothing was there */
- if (enetc_rd_reg(®s->mdio_cfg) & MDIO_CFG_RD_ER) {
+ if (enetc_mdio_rd(hw, MDIO_CFG) & MDIO_CFG_RD_ER) {
dev_dbg(&bus->dev,
"Error while reading PHY%d reg at %d.%hhu\n",
phy_id, dev_addr, regnum);
return 0xffff;
}
- value = enetc_rd_reg(®s->mdio_data) & 0xffff;
+ value = enetc_mdio_rd(hw, MDIO_DATA) & 0xffff;
return value;
}
@@ -146,12 +154,12 @@ static int enetc_mdio_read(struct mii_bus *bus, int phy_id, int regnum)
int enetc_mdio_probe(struct enetc_pf *pf)
{
struct device *dev = &pf->si->pdev->dev;
- struct enetc_mdio_regs __iomem *regs;
+ struct enetc_mdio_priv *mdio_priv;
struct device_node *np;
struct mii_bus *bus;
- int ret;
+ int err;
- bus = mdiobus_alloc_size(sizeof(regs));
+ bus = devm_mdiobus_alloc_size(dev, sizeof(*mdio_priv));
if (!bus)
return -ENOMEM;
@@ -159,41 +167,31 @@ int enetc_mdio_probe(struct enetc_pf *pf)
bus->read = enetc_mdio_read;
bus->write = enetc_mdio_write;
bus->parent = dev;
+ mdio_priv = bus->priv;
+ mdio_priv->hw = &pf->si->hw;
snprintf(bus->id, MII_BUS_ID_SIZE, "%s", dev_name(dev));
- /* store the enetc mdio base address for this bus */
- regs = pf->si->hw.port + ENETC_MDIO_REG_OFFSET;
- bus->priv = regs;
-
np = of_get_child_by_name(dev->of_node, "mdio");
if (!np) {
dev_err(dev, "MDIO node missing\n");
- ret = -EINVAL;
- goto err_registration;
+ return -EINVAL;
}
- ret = of_mdiobus_register(bus, np);
- if (ret) {
+ err = of_mdiobus_register(bus, np);
+ if (err) {
of_node_put(np);
dev_err(dev, "cannot register MDIO bus\n");
- goto err_registration;
+ return err;
}
of_node_put(np);
pf->mdio = bus;
return 0;
-
-err_registration:
- mdiobus_free(bus);
-
- return ret;
}
void enetc_mdio_remove(struct enetc_pf *pf)
{
- if (pf->mdio) {
+ if (pf->mdio)
mdiobus_unregister(pf->mdio);
- mdiobus_free(pf->mdio);
- }
}
--
2.17.1
^ permalink raw reply related
* [PATCH net-next v3 2/4] enetc: Add mdio bus driver for the PCIe MDIO endpoint
From: Claudiu Manoil @ 2019-07-29 10:03 UTC (permalink / raw)
To: David S . Miller
Cc: andrew, Rob Herring, Li Yang, alexandru.marginean, netdev,
devicetree, linux-arm-kernel, linux-kernel
In-Reply-To: <1564394627-3810-1-git-send-email-claudiu.manoil@nxp.com>
ENETC ports can manage the MDIO bus via local register
interface. However there's also a centralized way
to manage the MDIO bus, via the MDIO PCIe endpoint
device integrated by the same root complex that also
integrates the ENETC ports (eth controllers).
Depending on board design and use case, centralized
access to MDIO may be better than using local ENETC
port registers. For instance, on the LS1028A QDS board
where MDIO muxing is required. Also, the LS1028A on-chip
switch doesn't have a local MDIO register interface.
The current patch registers the above PCIe endpoint as a
separate MDIO bus and provides a driver for it by re-using
the code used for local MDIO access. It also allows the
ENETC port PHYs to be managed by this driver if the local
"mdio" node is missing from the ENETC port node.
Signed-off-by: Claudiu Manoil <claudiu.manoil@nxp.com>
---
v1 - fixed mdio bus allocation
- requested only BAR0 region, as it's the only one used by the driver
v2 - reworked accessors as per Andrew Lunn's request
v3 - none
.../net/ethernet/freescale/enetc/enetc_mdio.c | 98 +++++++++++++++++++
.../net/ethernet/freescale/enetc/enetc_pf.c | 5 +-
2 files changed, 102 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/freescale/enetc/enetc_mdio.c b/drivers/net/ethernet/freescale/enetc/enetc_mdio.c
index 05094601ece8..56ad94a3504c 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc_mdio.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc_mdio.c
@@ -195,3 +195,101 @@ void enetc_mdio_remove(struct enetc_pf *pf)
if (pf->mdio)
mdiobus_unregister(pf->mdio);
}
+
+#define ENETC_MDIO_DEV_ID 0xee01
+#define ENETC_MDIO_DEV_NAME "FSL PCIe IE Central MDIO"
+#define ENETC_MDIO_BUS_NAME ENETC_MDIO_DEV_NAME " Bus"
+#define ENETC_MDIO_DRV_NAME ENETC_MDIO_DEV_NAME " driver"
+#define ENETC_MDIO_DRV_ID "fsl_enetc_mdio"
+
+static int enetc_pci_mdio_probe(struct pci_dev *pdev,
+ const struct pci_device_id *ent)
+{
+ struct enetc_mdio_priv *mdio_priv;
+ struct device *dev = &pdev->dev;
+ struct enetc_hw *hw;
+ struct mii_bus *bus;
+ int err;
+
+ hw = devm_kzalloc(dev, sizeof(*hw), GFP_KERNEL);
+ if (!hw)
+ return -ENOMEM;
+
+ bus = devm_mdiobus_alloc_size(dev, sizeof(*mdio_priv));
+ if (!bus)
+ return -ENOMEM;
+
+ bus->name = ENETC_MDIO_BUS_NAME;
+ bus->read = enetc_mdio_read;
+ bus->write = enetc_mdio_write;
+ bus->parent = dev;
+ mdio_priv = bus->priv;
+ mdio_priv->hw = hw;
+ snprintf(bus->id, MII_BUS_ID_SIZE, "%s", dev_name(dev));
+
+ pcie_flr(pdev);
+ err = pci_enable_device_mem(pdev);
+ if (err) {
+ dev_err(dev, "device enable failed\n");
+ return err;
+ }
+
+ err = pci_request_region(pdev, 0, ENETC_MDIO_DRV_ID);
+ if (err) {
+ dev_err(dev, "pci_request_region failed\n");
+ goto err_pci_mem_reg;
+ }
+
+ hw->port = pci_iomap(pdev, 0, 0);
+ if (!bus->priv) {
+ err = -ENXIO;
+ dev_err(dev, "iomap failed\n");
+ goto err_ioremap;
+ }
+
+ err = of_mdiobus_register(bus, dev->of_node);
+ if (err)
+ goto err_mdiobus_reg;
+
+ pci_set_drvdata(pdev, bus);
+
+ return 0;
+
+err_mdiobus_reg:
+ iounmap(mdio_priv->hw->port);
+err_ioremap:
+ pci_release_mem_regions(pdev);
+err_pci_mem_reg:
+ pci_disable_device(pdev);
+
+ return err;
+}
+
+static void enetc_pci_mdio_remove(struct pci_dev *pdev)
+{
+ struct mii_bus *bus = pci_get_drvdata(pdev);
+ struct enetc_mdio_priv *mdio_priv;
+
+ mdiobus_unregister(bus);
+ mdio_priv = bus->priv;
+ iounmap(mdio_priv->hw->port);
+ pci_release_mem_regions(pdev);
+ pci_disable_device(pdev);
+}
+
+static const struct pci_device_id enetc_pci_mdio_id_table[] = {
+ { PCI_DEVICE(PCI_VENDOR_ID_FREESCALE, ENETC_MDIO_DEV_ID) },
+ { 0, } /* End of table. */
+};
+MODULE_DEVICE_TABLE(pci, enetc_mdio_id_table);
+
+static struct pci_driver enetc_pci_mdio_driver = {
+ .name = ENETC_MDIO_DRV_ID,
+ .id_table = enetc_pci_mdio_id_table,
+ .probe = enetc_pci_mdio_probe,
+ .remove = enetc_pci_mdio_remove,
+};
+module_pci_driver(enetc_pci_mdio_driver);
+
+MODULE_DESCRIPTION(ENETC_MDIO_DRV_NAME);
+MODULE_LICENSE("Dual BSD/GPL");
diff --git a/drivers/net/ethernet/freescale/enetc/enetc_pf.c b/drivers/net/ethernet/freescale/enetc/enetc_pf.c
index 258b3cb38a6f..7d6513ff8507 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc_pf.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc_pf.c
@@ -750,6 +750,7 @@ static int enetc_of_get_phy(struct enetc_ndev_priv *priv)
{
struct enetc_pf *pf = enetc_si_priv(priv->si);
struct device_node *np = priv->dev->of_node;
+ struct device_node *mdio_np;
int err;
if (!np) {
@@ -773,7 +774,9 @@ static int enetc_of_get_phy(struct enetc_ndev_priv *priv)
priv->phy_node = of_node_get(np);
}
- if (!of_phy_is_fixed_link(np)) {
+ mdio_np = of_get_child_by_name(np, "mdio");
+ if (mdio_np) {
+ of_node_put(mdio_np);
err = enetc_mdio_probe(pf);
if (err) {
of_node_put(priv->phy_node);
--
2.17.1
^ permalink raw reply related
* [PATCH net-next v3 3/4] dt-bindings: net: fsl: enetc: Add bindings for the central MDIO PCIe endpoint
From: Claudiu Manoil @ 2019-07-29 10:03 UTC (permalink / raw)
To: David S . Miller
Cc: andrew, Rob Herring, Li Yang, alexandru.marginean, netdev,
devicetree, linux-arm-kernel, linux-kernel
In-Reply-To: <1564394627-3810-1-git-send-email-claudiu.manoil@nxp.com>
The on-chip PCIe root complex that integrates the ENETC ethernet
controllers also integrates a PCIe endpoint for the MDIO controller
providing for centralized control of the ENETC mdio bus.
Add bindings for this "central" MDIO Integrated PCIe Endpoint.
Signed-off-by: Claudiu Manoil <claudiu.manoil@nxp.com>
---
v1 - none
v2 - none
v3 - fixed spelling (commit message mostly)
.../devicetree/bindings/net/fsl-enetc.txt | 42 +++++++++++++++++--
1 file changed, 39 insertions(+), 3 deletions(-)
diff --git a/Documentation/devicetree/bindings/net/fsl-enetc.txt b/Documentation/devicetree/bindings/net/fsl-enetc.txt
index 25fc687419db..b7034ccbc1bd 100644
--- a/Documentation/devicetree/bindings/net/fsl-enetc.txt
+++ b/Documentation/devicetree/bindings/net/fsl-enetc.txt
@@ -11,7 +11,9 @@ Required properties:
to parent node bindings.
- compatible : Should be "fsl,enetc".
-1) The ENETC external port is connected to a MDIO configurable phy:
+1. The ENETC external port is connected to a MDIO configurable phy
+
+1.1. Using the local ENETC Port MDIO interface
In this case, the ENETC node should include a "mdio" sub-node
that in turn should contain the "ethernet-phy" node describing the
@@ -47,8 +49,42 @@ Example:
};
};
-2) The ENETC port is an internal port or has a fixed-link external
-connection:
+1.2. Using the central MDIO PCIe endpoint device
+
+In this case, the mdio node should be defined as another PCIe
+endpoint node, at the same level with the ENETC port nodes.
+
+Required properties:
+
+- reg : Specifies PCIe Device Number and Function
+ Number of the ENETC endpoint device, according
+ to parent node bindings.
+- compatible : Should be "fsl,enetc-mdio".
+
+The remaining required mdio bus properties are standard, their bindings
+already defined in Documentation/devicetree/bindings/net/mdio.txt.
+
+Example:
+
+ ethernet@0,0 {
+ compatible = "fsl,enetc";
+ reg = <0x000000 0 0 0 0>;
+ phy-handle = <&sgmii_phy0>;
+ phy-connection-type = "sgmii";
+ };
+
+ mdio@0,3 {
+ compatible = "fsl,enetc-mdio";
+ reg = <0x000300 0 0 0 0>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ sgmii_phy0: ethernet-phy@2 {
+ reg = <0x2>;
+ };
+ };
+
+2. The ENETC port is an internal port or has a fixed-link external
+connection
In this case, the ENETC port node defines a fixed link connection,
as specified by Documentation/devicetree/bindings/net/fixed-link.txt.
--
2.17.1
^ permalink raw reply related
* [PATCH net-next v3 4/4] arm64: dts: fsl: ls1028a: Enable eth port1 on the ls1028a QDS board
From: Claudiu Manoil @ 2019-07-29 10:03 UTC (permalink / raw)
To: David S . Miller
Cc: andrew, Rob Herring, Li Yang, alexandru.marginean, netdev,
devicetree, linux-arm-kernel, linux-kernel
In-Reply-To: <1564394627-3810-1-git-send-email-claudiu.manoil@nxp.com>
LS1028a has one Ethernet management interface. On the QDS board, the
MDIO signals are multiplexed to either on-board AR8035 PHY device or
to 4 PCIe slots allowing for SGMII cards.
To enable the Ethernet ENETC Port 1, which can only be connected to a
RGMII PHY, the multiplexer needs to be configured to route the MDIO to
the AR8035 PHY. The MDIO/MDC routing is controlled by bits 7:4 of FPGA
board config register 0x54, and value 0 selects the on-board RGMII PHY.
The FPGA board config registers are accessible on the i2c bus, at address
0x66.
The PF3 MDIO PCIe integrated endpoint device allows for centralized access
to the MDIO bus. Add the corresponding devicetree node and set it to be
the MDIO bus parent.
Signed-off-by: Alex Marginean <alexandru.marginean@nxp.com>
Signed-off-by: Claudiu Manoil <claudiu.manoil@nxp.com>
---
v1 - none
v2 - none
v3 - none
.../boot/dts/freescale/fsl-ls1028a-qds.dts | 40 +++++++++++++++++++
.../arm64/boot/dts/freescale/fsl-ls1028a.dtsi | 6 +++
2 files changed, 46 insertions(+)
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1028a-qds.dts b/arch/arm64/boot/dts/freescale/fsl-ls1028a-qds.dts
index de6ef39f3118..663c4b728c07 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1028a-qds.dts
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1028a-qds.dts
@@ -85,6 +85,26 @@
system-clock-frequency = <25000000>;
};
};
+
+ mdio-mux {
+ compatible = "mdio-mux-multiplexer";
+ mux-controls = <&mux 0>;
+ mdio-parent-bus = <&enetc_mdio_pf3>;
+ #address-cells=<1>;
+ #size-cells = <0>;
+
+ /* on-board RGMII PHY */
+ mdio@0 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0>;
+
+ qds_phy1: ethernet-phy@5 {
+ /* Atheros 8035 */
+ reg = <5>;
+ };
+ };
+ };
};
&duart0 {
@@ -164,6 +184,26 @@
};
};
};
+
+ fpga@66 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ compatible = "fsl,ls1028aqds-fpga", "fsl,fpga-qixis-i2c",
+ "simple-mfd";
+ reg = <0x66>;
+
+ mux: mux-controller {
+ compatible = "reg-mux";
+ #mux-control-cells = <1>;
+ mux-reg-masks = <0x54 0xf0>; /* 0: reg 0x54, bits 7:4 */
+ };
+ };
+
+};
+
+&enetc_port1 {
+ phy-handle = <&qds_phy1>;
+ phy-connection-type = "rgmii-id";
};
&sai1 {
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
index 7975519b4f56..de71153fda00 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
@@ -536,6 +536,12 @@
compatible = "fsl,enetc";
reg = <0x000100 0 0 0 0>;
};
+ enetc_mdio_pf3: mdio@0,3 {
+ compatible = "fsl,enetc-mdio";
+ reg = <0x000300 0 0 0 0>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ };
ethernet@0,4 {
compatible = "fsl,enetc-ptp";
reg = <0x000400 0 0 0 0>;
--
2.17.1
^ permalink raw reply related
* Re: [RFC,v3 6/9] media: platform: Add Mediatek ISP P1 V4L2 functions
From: Tomasz Figa @ 2019-07-29 10:04 UTC (permalink / raw)
To: Jungo Lin
Cc: devicetree, Sean Cheng (鄭昇弘),
Mauro Carvalho Chehab, Rynn Wu (吳育恩),
srv_heupstream, Rob Herring, Ryan Yu (余孟修),
Frankie Chiu (邱文凱), Hans Verkuil,
Matthias Brugger, Sj Huang,
moderated list:ARM/Mediatek SoC support, Laurent Pinchart,
ddavenport, Frederic Chen (陳俊元)
In-Reply-To: <1564363089.1212.636.camel@mtksdccf07>
On Mon, Jul 29, 2019 at 10:18 AM Jungo Lin <jungo.lin@mediatek.com> wrote:
> On Fri, 2019-07-26 at 14:49 +0900, Tomasz Figa wrote:
> > On Wed, Jul 24, 2019 at 1:31 PM Jungo Lin <jungo.lin@mediatek.com> wrote:
> > > On Tue, 2019-07-23 at 19:21 +0900, Tomasz Figa wrote:
> > > > On Thu, Jul 18, 2019 at 1:39 PM Jungo Lin <jungo.lin@mediatek.com> wrote:
> > > > > On Wed, 2019-07-10 at 18:54 +0900, Tomasz Figa wrote:
> > > > > > On Tue, Jun 11, 2019 at 11:53:41AM +0800, Jungo Lin wrote:
[snip]
> > > dev_dbg(cam->dev, "jobs are full\n");
> > > spin_unlock_irqrestore(&cam->pending_job_lock, flags);
> > > return;
> > > }
> > > list_for_each_entry_safe(req, req_prev, &cam->pending_job_list, list) {
> >
> > Could we instead check the counter here and break if it's >=
> > MTK_ISP_MAX_RUNNING_JOBS?
> > Then we could increment it here too to simplify the code.
> >
>
> Thanks for your advice.
> We simplified this function as below:
>
> void mtk_cam_dev_req_try_queue(struct mtk_cam_dev *cam)
> {
> struct mtk_cam_dev_request *req, *req_prev;
> unsigned long flags;
>
> if (!cam->streaming) {
> dev_dbg(cam->dev, "stream is off\n");
> return;
> }
>
> spin_lock_irq(&cam->pending_job_lock);
> spin_lock_irqsave(&cam->running_job_lock, flags);
Having the inner call spin_lock_irqsave() doesn't really do anything
useful, because the outer spin_lock_irq() disables the IRQs and flags
would always have the IRQ disabled state. Please use irqsave for the
outer call.
[snip]
> > > > > > > +
> > > > > > > +static struct v4l2_subdev *
> > > > > > > +mtk_cam_cio_get_active_sensor(struct mtk_cam_dev *cam_dev)
> > > > > > > +{
> > > > > > > + struct media_device *mdev = cam_dev->seninf->entity.graph_obj.mdev;
> > > > > > > + struct media_entity *entity;
> > > > > > > + struct device *dev = &cam_dev->pdev->dev;
> > > > > > > + struct v4l2_subdev *sensor;
> > > > > >
> > > > > > This variable would be unitialized if there is no streaming sensor. Was
> > > > > > there no compiler warning generated for this?
> > > > > >
> > > > >
> > > > > No, there is no compiler warning.
> > > > > But, we will assign sensor to NULL to avoid unnecessary compiler warning
> > > > > with different compiler options.
> > > > >
> > > >
> > > > Thanks. It would be useful if you could check why the compiler you're
> > > > using doesn't show a warning here. We might be missing other
> > > > uninitialized variables.
> > > >
> > >
> > > We will feedback to your project team to check the possible reason about
> > > compiler warning issue.
> > >
> >
> > Do you mean that it was the Clang toolchain used on Chromium OS (e.g.
> > emerge chromeos-kernel-4_19)?
>
> > [snip]
>
> Yes, I checked this comment in the Chromium OS build environment.
> But, I think I have made the mistake here. I need to check the build
> status in the Mediatek's kernel upstream environment. I will pay
> attention in next path set upstream.
>
Thanks a lot. I will recheck this in the Chromium OS toolchain too.
> > > > > > > +
> > > > > > > + dev_dbg(dev, "%s: node:%d fd:%d idx:%d\n",
> > > > > > > + __func__,
> > > > > > > + node->id,
> > > > > > > + buf->vbb.request_fd,
> > > > > > > + buf->vbb.vb2_buf.index);
> > > > > > > +
> > > > > > > + /* For request buffers en-queue, handled in mtk_cam_req_try_queue */
> > > > > > > + if (vb->vb2_queue->uses_requests)
> > > > > > > + return;
> > > > > >
> > > > > > I'd suggest removing non-request support from this driver. Even if we end up
> > > > > > with a need to provide compatibility for non-request mode, then it should be
> > > > > > built on top of the requests mode, so that the driver itself doesn't have to
> > > > > > deal with two modes.
> > > > > >
> > > > >
> > > > > The purpose of non-request function in this driver is needed by
> > > > > our camera middle-ware design. It needs 3A statistics buffers before
> > > > > image buffers en-queue. So we need to en-queue 3A statistics with
> > > > > non-request mode in this driver. After MW got the 3A statistics data, it
> > > > > will en-queue the images, tuning buffer and other meta buffers with
> > > > > request mode. Based on this requirement, do you have any suggestion?
> > > > > For upstream driver, should we only consider request mode?
> > > > >
> > > >
> > > > Where does that requirement come from? Why the timing of queuing of
> > > > the buffers to the driver is important?
> > > >
> > > > [snip]
> > >
> > > Basically, this requirement comes from our internal camera
> > > middle-ware/3A hal in user space. Since this is not generic requirement,
> > > we will follow your original suggestion to keep the request mode only
> > > and remove other non-request design in other files. For upstream driver,
> > > it should support request mode only.
> > >
> >
> > Note that Chromium OS will use the "upstream driver" and we don't want
> > to diverge, so please make the userspace also use only requests. I
> > don't see a reason why there would be any need to submit any buffers
> > outside of a request.
> >
> > [snip]
>
> Ok, I have raised your concern to our colleagues and let him to discuss
> with you in another communication channel.
>
Thanks!
Best regards,
Tomasz
^ permalink raw reply
* Re: [PATCH 2/2] arm64: dts: amazon: add Amazon Annapurna Labs Alpine v3 support
From: Sudeep Holla @ 2019-07-29 10:12 UTC (permalink / raw)
To: Ronen Krupnik
Cc: robh+dt, mark.rutland, devicetree, linux-kernel, barakw, dwmw,
benh, jonnyc, talel, hhhawa, hanochu, Sudeep Holla
In-Reply-To: <20190728195135.12661-3-ronenk@amazon.com>
On Sun, Jul 28, 2019 at 10:51:35PM +0300, Ronen Krupnik wrote:
> This patch adds the initial support for the Amazon Annapurna Labs Alpine v3
> Soc and Evaluation Platform (EVP).
[...]
> +
> + pmu {
> + compatible = "arm,armv8-pmuv3";
Please use "arm,cortex-a72-pmu" as we know it's Cortex-A72 cores
--
Regards,
Sudeep
^ permalink raw reply
* Re: [PATCH 2/6] pwm: sun4i: Add a quirk for reset line
From: Philipp Zabel @ 2019-07-29 10:18 UTC (permalink / raw)
To: Uwe Kleine-König, Chen-Yu Tsai
Cc: Jernej Skrabec, Thierry Reding, Maxime Ripard, Rob Herring,
Mark Rutland, linux-pwm, devicetree, linux-arm-kernel,
linux-kernel, linux-sunxi
In-Reply-To: <20190729071218.bukw7vxilqy523k3@pengutronix.de>
Hi,
On Mon, 2019-07-29 at 09:12 +0200, Uwe Kleine-König wrote:
> Hello,
>
> On Mon, Jul 29, 2019 at 02:43:23PM +0800, Chen-Yu Tsai wrote:
> > On Mon, Jul 29, 2019 at 2:36 PM Uwe Kleine-König
> > <u.kleine-koenig@pengutronix.de> wrote:
> > > On Fri, Jul 26, 2019 at 08:40:41PM +0200, Jernej Skrabec wrote:
> > > > @@ -371,6 +374,14 @@ static int sun4i_pwm_probe(struct platform_device *pdev)
> > > > if (IS_ERR(pwm->clk))
> > > > return PTR_ERR(pwm->clk);
> > > >
> > > > + if (pwm->data->has_reset) {
> > > > + pwm->rst = devm_reset_control_get(&pdev->dev, NULL);
> > > > + if (IS_ERR(pwm->rst))
> > > > + return PTR_ERR(pwm->rst);
> > > > +
> > > > + reset_control_deassert(pwm->rst);
> > > > + }
> > > > +
> > >
> > > I wonder why there is a need to track if a given chip needs a reset
> > > line. I'd just use devm_reset_control_get_optional() and drop the
> > > .has_reset member in struct sun4i_pwm_data.
> >
> > Because it's not optional for this platform, i.e. it won't work if
> > the reset control (or clk, in the next patch) is somehow missing from
> > the device tree.
>
> If the device tree is wrong it is considered ok that the driver doesn't
> behave correctly. So this is not a problem you need (or should) care
> about.
I agree with this. Catching missing DT properties and other device tree
validation is not the job of device drivers. The _optional request
variants were introduced to simplify drivers that require the reset line
on some platforms and not on others.
I would ask to explicitly state whether the driver needs full control
over the moment of (de)assertion of the reset signal, or whether the
only requirement is that the reset signal stays deasserted while the PWM
driver is active, by using devm_reset_control_get_optional_exclusive or
devm_reset_control_get_optional_shared to request the reset control.
regards
Philipp
^ permalink raw reply
* Re: [PATCH v2 2/2] media: tc358746: add Toshiba TC358746 Parallel to CSI-2 bridge driver
From: Marco Felsch @ 2019-07-29 10:40 UTC (permalink / raw)
To: Sakari Ailus
Cc: mchehab, robh+dt, Jacopo Mondi, devicetree, graphics, linux-media
In-Reply-To: <20190625122719.xcl3gxxs4gpuvetf@paasikivi.fi.intel.com>
Hi Sakari,
On 19-06-25 15:27, Sakari Ailus wrote:
> Hi Marco,
>
> Thanks for the set.
>
> Looks quite good to me in general; a few minor comments below.
Sorry for the delayed response.
> On Wed, Jun 19, 2019 at 05:28:38PM +0200, Marco Felsch wrote:
> > Adding support for the TC358746 bridge. The Bridge can receive images on
> > the parallel input port and send it to the host using the CSI-TX unit.
> > Furthermore the Bridge can receive images from the host using the CSI-RX
> > unit and send it to the parallel output port.
> >
> > Currently the only the first case is implemented and tested. The bridge
> > driver needs two information from the connected sensor: hblank time and
> > pixel-rate. Both information are requested using the v4l2_ctrl interface.
> > The driver won't create a media-link if one or both information are
> > missing.
> >
> > Missing feature:
> > - Provide mclk on GPIO[0]
> > - Sending pictures from the host to a parallel display
> > - v4l_event support
> >
> > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> > ---
> > Changes
> > v2:
> > - adapt Kconfig symbole description
> > - reorder includes alphabetical
> > - drop refclk input-clock is named 'refclk'
> > - fix module license to GPL v2
> > - use unsigned int for zero-based count vars
> > - use unsigned int for width parameter in tc358746_adjust_fifo_size()
> > tc358746_adjust_timings()
> > - use unsgined int for state->link_freq local variables since that
> > can't be negative
> > - drop tc358746_g_mbus_config() since it will be removed soon
> > - store the sensor_pclk_ctrl and sensor_hblank_ctrl references to
> > request the ctrl's only once.
> > - replace dev_*() by v4l2_*() macros
> > - move to probe_new() and drop i2c_device_id table
> > - improve error path during probe()
> > - change i2c_wr16_and_or() logic so the user only have to specify the
> > mask. This avoids build warnings due to '-Woverflow' compilations.
> > The bug was found by the kbuild test robot.
> >
> > drivers/media/i2c/Kconfig | 12 +
> > drivers/media/i2c/Makefile | 1 +
> > drivers/media/i2c/tc358746.c | 1807 +++++++++++++++++++++++++++++
> > drivers/media/i2c/tc358746_regs.h | 208 ++++
> > 4 files changed, 2028 insertions(+)
> > create mode 100644 drivers/media/i2c/tc358746.c
> > create mode 100644 drivers/media/i2c/tc358746_regs.h
> >
> > diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
> > index cb8db944aa41..454d8f682126 100644
> > --- a/drivers/media/i2c/Kconfig
> > +++ b/drivers/media/i2c/Kconfig
> > @@ -360,6 +360,18 @@ config VIDEO_TC358743_CEC
> > When selected the tc358743 will support the optional
> > HDMI CEC feature.
> >
> > +config VIDEO_TC358746
> > + tristate "Toshiba TC358746 parallel-CSI2 bridge"
> > + depends on VIDEO_V4L2 && I2C && VIDEO_V4L2_SUBDEV_API && MEDIA_CONTROLLER
> > + select V4L2_FWNODE
> > + help
> > + Support for the Toshiba TC358746 PARALLEL to MIPI CSI-2 bridge.
> > + The bridge can work in both directions but currenty only the
>
> "currently"
Fixed, thanks.
> > + parallel-in / csi-out path is supported.
> > +
> > + To compile this driver as a module, choose M here: the
> > + module will be called tc358746.
> > +
> > config VIDEO_TVP514X
> > tristate "Texas Instruments TVP514x video decoder"
> > depends on VIDEO_V4L2 && I2C
> > diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
> > index d8ad9dad495d..3d5073cc26cf 100644
> > --- a/drivers/media/i2c/Makefile
> > +++ b/drivers/media/i2c/Makefile
> > @@ -108,6 +108,7 @@ obj-$(CONFIG_VIDEO_I2C) += video-i2c.o
> > obj-$(CONFIG_VIDEO_ML86V7667) += ml86v7667.o
> > obj-$(CONFIG_VIDEO_OV2659) += ov2659.o
> > obj-$(CONFIG_VIDEO_TC358743) += tc358743.o
> > +obj-$(CONFIG_VIDEO_TC358746) += tc358746.o
> > obj-$(CONFIG_VIDEO_IMX214) += imx214.o
> > obj-$(CONFIG_VIDEO_IMX258) += imx258.o
> > obj-$(CONFIG_VIDEO_IMX274) += imx274.o
> > diff --git a/drivers/media/i2c/tc358746.c b/drivers/media/i2c/tc358746.c
> > new file mode 100644
> > index 000000000000..58612824b267
> > --- /dev/null
> > +++ b/drivers/media/i2c/tc358746.c
> > @@ -0,0 +1,1807 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * tc358746 - Parallel to CSI-2 bridge
> > + *
> > + * Copyright 2018 Marco Felsch <kernel@pengutronix.de>
> > + *
> > + * References:
> > + * REF_01:
> > + * - TC358746AXBG/TC358748XBG/TC358748IXBG Functional Specification Rev 1.2
> > + * REF_02:
> > + * - TC358746(A)748XBG_Parallel-CSI2_Tv23p.xlsx, Rev Tv23
> > + */
> > +
> > +#include <linux/clk.h>
> > +#include <linux/clk-provider.h>
> > +#include <linux/delay.h>
> > +#include <linux/gpio/consumer.h>
> > +#include <linux/i2c.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/kernel.h>
> > +#include <linux/module.h>
> > +#include <linux/property.h>
> > +#include <linux/slab.h>
> > +#include <linux/timer.h>
> > +#include <media/v4l2-ctrls.h>
> > +#include <media/v4l2-device.h>
> > +#include <media/v4l2-fwnode.h>
> > +
> > +#include "tc358746_regs.h"
> > +
> > +static int debug;
> > +module_param(debug, int, 0644);
> > +MODULE_PARM_DESC(debug, "debug level (0-3)");
> > +
> > +MODULE_DESCRIPTION("Toshiba TC358746 Parallel to CSI-2 bridge driver");
> > +MODULE_AUTHOR("Marco Felsch <kernel@pengutronix.de>");
> > +MODULE_LICENSE("GPL v2");
>
> These are usually placed at the end.
Okay, I will move it at the end.
> > +
> > +#define I2C_MAX_XFER_SIZE (512 + 2)
> > +#define TC358746_MAX_FIFO_SIZE 512
> > +#define TC358746_DEF_LINK_FREQ 0
> > +
> > +#define TC358746_LINEINIT_MIN_US 110
> > +#define TC358746_TWAKEUP_MIN_US 1200
> > +#define TC358746_LPTXTIME_MIN_NS 55
> > +#define TC358746_TCLKZERO_MIN_NS 305
> > +#define TC358746_TCLKTRAIL_MIN_NS 65
> > +#define TC358746_TCLKPOST_MIN_NS 65
> > +#define TC358746_THSZERO_MIN_NS 150
> > +#define TC358746_THSTRAIL_MIN_NS 65
> > +#define TC358746_THSPREPARE_MIN_NS 45
> > +
> > +static const struct v4l2_mbus_framefmt tc358746_def_fmt = {
> > + .width = 640,
> > + .height = 480,
> > + .code = MEDIA_BUS_FMT_UYVY8_2X8,
> > + .field = V4L2_FIELD_NONE,
> > + .colorspace = V4L2_COLORSPACE_DEFAULT,
> > + .ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT,
> > + .quantization = V4L2_QUANTIZATION_DEFAULT,
> > + .xfer_func = V4L2_XFER_FUNC_DEFAULT,
> > +};
> > +
> > +struct tc358746_csi_param {
> > + unsigned char speed_range;
> > + unsigned int unit_clk_hz;
> > + unsigned char unit_clk_mul;
> > + unsigned int speed_per_lane; /* bps / lane */
> > + unsigned short lane_num;
> > + bool is_continuous_clk;
> > +
> > + /* CSI2-TX Parameters */
> > + u32 lineinitcnt;
> > + u32 lptxtimecnt;
> > + u32 twakeupcnt;
> > + u32 tclk_preparecnt;
> > + u32 tclk_zerocnt;
> > + u32 tclk_trailcnt;
> > + u32 tclk_postcnt;
> > + u32 ths_preparecnt;
> > + u32 ths_zerocnt;
> > + u32 ths_trailcnt;
> > +
> > + unsigned int csi_hs_lp_hs_ps;
> > +};
>
> Would it be possible to use struct phy_configure_opts_mipi_dphy and perhaps
> phy_mipi_dphy_get_default_config() as well? The result of the latter will
> surely be different though, but still expected to be valid.
I've checked the struct phy_configure_opts_mipi_dphy and have a few
points to think about:
1) The values I need here are counter values. The struct you mentioned
uses time based values. This is good but needs a retranslation into
the count values.
2) The tclk_zerocnt and tclk_zerocnt has a other meaning here:
tclk_zerocnt = ckl_prepare + clk_zero > 300ns
ths_zerocnt = hs_prepare + hs_zero > 145ns + 10 * ui
I can use the struct phy_configure_opts_mipi_dphy but then the
meaning are not the same..
If I should use the phy_mipi_dphy_get_default_config() helper I had to
reconstruct the whole logic. Currently the driver is based on the
following behaviour:
- the fw (DT in our case) defines possible link frequencies and data
lane numbers
- based on that information I calculate possible counter values to
confirm the mipi-spec
- now the user specifies a format and I try to serve the request by
- adjusting the fifo-size
if not possible
- use other link frequency setting and retry adjusting fifo_size
if till not possible
- begin to reduce the resultion
Imagine it that way: I configure the dphy side and adjust the parallel
side.
Now phy_mipi_dphy_get_default_config() uses the format bpp and the
pixel_clk to calculate the exact default config. Those configs must be
converted to the counter values. This wouldn't be a big deal but the
excel-sheet I used to calculate the timings adds some "random"
undocumented constants. So it would not be just a conversion from a
time-val to a counter-val. After that we need to verify the
parallel-input settings against the dphy-settings and do proper
fifo/picture-soze adjustments.
I would keep my approach beacause of the "random" undocumented constants
and the other meaning of the struct phy_configure_opts_mipi_dphy
members. Maybe I can convert it later if I have more time to verify that
the "random" undocumented constants aren't important and can be dropped.
Also I checked a few other Toshiba TC convert chips and it seems that
all off them uses the same DPHY. So maybe we should split out the
phy-part to share it.
> > +
> > +enum tc358746_csi_direction {
> > + TC358746_CSI_RX, /* CSI-in -> Parallel-out */
> > + TC358746_CSI_TX /* Parallel-in -> CSI-out */
> > +};
> > +
> > +struct tc358746_state {
> > + struct v4l2_subdev sd;
> > + struct i2c_client *i2c_client;
> > + struct gpio_desc *reset_gpio;
> > +
> > + /*
> > + * Generic
> > + */
> > + struct media_pad pads[2];
> > + struct mutex confctl_mutex;
> > + struct v4l2_mbus_framefmt fmt;
> > + struct v4l2_ctrl_handler hdl;
> > + bool fmt_changed;
> > + bool test;
> > +
> > + /*
> > + * Chip Clocks
> > + */
> > + struct clk *refclk;
> > + /* internal pll */
> > + unsigned int pllinclk_hz;
> > + u16 pll_prd;
> > + u16 pll_fbd;
> > +
> > + /*
> > + * Video Buffer
> > + */
> > + u16 vb_fifo; /* The FIFO size is 511x32 */
> > +
> > + /* currently only TC358746_CSI_TX supported */
> > + enum tc358746_csi_direction csi_dir;
> > +
> > + /*
> > + * CSI TX
> > + */
> > + struct v4l2_ctrl *link_freq;
> > + struct tc358746_csi_param *link_freq_settings;
> > + u64 *link_frequencies;
> > + unsigned int link_frequencies_num;
> > +
> > + /*
> > + * Parallel input
> > + */
> > + struct v4l2_ctrl *sensor_pclk_ctrl;
> > + struct v4l2_ctrl *sensor_hblank_ctrl;
> > + unsigned int pclk;
> > + unsigned int hblank;
> > +};
> > +
> > +struct tc358746_mbus_fmt {
> > + u32 code;
> > + u8 bus_width;
> > + u8 bpp; /* total bpp */
> > + u8 pdformat; /* peripheral data format */
> > + u8 pdataf; /* parallel data format option */
> > + u8 ppp; /* pclk per pixel */
> > + bool csitx_only; /* format only in csi-tx mode supported */
> > +};
> > +
> > +/* TODO: Add other formats as required */
> > +static const struct tc358746_mbus_fmt tc358746_formats[] = {
> > + {
> > + .code = MEDIA_BUS_FMT_UYVY8_2X8,
>
> Note that on CSI-2 V4L2 uses the one pixel / sample variants. I.e. this
> would be MEDIA_BUS_FMT_UYVY8_1X16.
Sorry but I didn't get you. The format you mentioned is...
> > + .bus_width = 8,
> > + .bpp = 16,
> > + .pdformat = DATAFMT_PDFMT_YCBCRFMT_422_8_BIT,
> > + .pdataf = CONFCTL_PDATAF_MODE0,
> > + .ppp = 2,
> > + }, {
> > + .code = MEDIA_BUS_FMT_UYVY8_1X16,
here.
> > + .bus_width = 16,
> > + .bpp = 16,
> > + .pdformat = DATAFMT_PDFMT_YCBCRFMT_422_8_BIT,
> > + .pdataf = CONFCTL_PDATAF_MODE1,
> > + .ppp = 1,
> > + }, {
> > + .code = MEDIA_BUS_FMT_YUYV8_1X16,
> > + .bus_width = 16,
> > + .bpp = 16,
> > + .pdformat = DATAFMT_PDFMT_YCBCRFMT_422_8_BIT,
> > + .pdataf = CONFCTL_PDATAF_MODE2,
> > + .ppp = 1,
> > + }, {
> > + .code = MEDIA_BUS_FMT_UYVY10_2X10,
> > + .bus_width = 10,
> > + .bpp = 20,
> > + .pdformat = DATAFMT_PDFMT_YCBCRFMT_422_10_BIT,
> > + .pdataf = CONFCTL_PDATAF_MODE0, /* don't care */
> > + .ppp = 2,
> > + }, {
> > + /* in datasheet listed as YUV444 */
> > + .code = MEDIA_BUS_FMT_GBR888_1X24,
> > + .bus_width = 24,
> > + .bpp = 24,
> > + .pdformat = DATAFMT_PDFMT_YCBCRFMT_444,
> > + .pdataf = CONFCTL_PDATAF_MODE0, /* don't care */
> > + .ppp = 2,
> > + .csitx_only = true,
> > + },
> > +};
> > +
> > +/* --------------- HELPERS ------------ */
> > +static void
> > +tc358746_dump_csi(struct v4l2_subdev *sd,
> > + struct tc358746_csi_param *csi_setting)
> > +{
> > + v4l2_dbg(2, debug, sd, "Speed-Range value %u\n",
> > + csi_setting->speed_range);
> > + v4l2_dbg(2, debug, sd, "Unit Clock %u Hz\n", csi_setting->unit_clk_hz);
> > + v4l2_dbg(2, debug, sd, "Unit Clock Mul %u\n",
> > + csi_setting->unit_clk_mul);
> > + v4l2_dbg(2, debug, sd, "CSI speed/lane %u bps/lane\n",
> > + csi_setting->speed_per_lane);
> > + v4l2_dbg(2, debug, sd, "CSI lanes %u\n", csi_setting->lane_num);
> > + v4l2_dbg(2, debug, sd, "CSI clock during LP %sabled\n",
> > + csi_setting->is_continuous_clk ? "en" : "dis");
> > +
> > + v4l2_dbg(2, debug, sd, "lineinitcnt %u\n", csi_setting->lineinitcnt);
> > + v4l2_dbg(2, debug, sd, "lptxtimecnt %u\n", csi_setting->lptxtimecnt);
> > + v4l2_dbg(2, debug, sd, "tclk_preparecnt %u\n",
> > + csi_setting->tclk_preparecnt);
> > + v4l2_dbg(2, debug, sd, "tclk_zerocnt %u\n", csi_setting->tclk_zerocnt);
> > + v4l2_dbg(2, debug, sd, "tclk_trailcnt %u\n",
> > + csi_setting->tclk_trailcnt);
> > + v4l2_dbg(2, debug, sd, "ths_preparecnt %u\n",
> > + csi_setting->ths_preparecnt);
> > + v4l2_dbg(2, debug, sd, "ths_zerocnt %u\n", csi_setting->ths_zerocnt);
> > + v4l2_dbg(2, debug, sd, "twakeupcnt %u\n", csi_setting->twakeupcnt);
> > + v4l2_dbg(2, debug, sd, "tclk_postcnt %u\n", csi_setting->tclk_postcnt);
> > + v4l2_dbg(2, debug, sd, "ths_trailcnt %u\n", csi_setting->ths_trailcnt);
> > + v4l2_dbg(2, debug, sd, "csi_hs_lp_hs_ps %u (%u us)\n",
> > + csi_setting->csi_hs_lp_hs_ps,
> > + csi_setting->csi_hs_lp_hs_ps / 1000);
> > +}
> > +
> > +static void
> > +tc358746_dump_pll(struct v4l2_subdev *sd, struct tc358746_state *state)
> > +{
> > + v4l2_dbg(2, debug, sd, "refclk %lu Hz\n", clk_get_rate(state->refclk));
> > + v4l2_dbg(2, debug, sd, "pll input clock %u Hz\n", state->pllinclk_hz);
> > + v4l2_dbg(2, debug, sd, "PLL_PRD %u\n", state->pll_prd - 1);
> > + v4l2_dbg(2, debug, sd, "PLL_FBD %u\n", state->pll_fbd - 1);
> > +}
> > +
> > +static inline struct tc358746_state *to_state(struct v4l2_subdev *sd)
> > +{
> > + return container_of(sd, struct tc358746_state, sd);
> > +}
> > +
> > +/* Find a data format by a pixel code */
> > +static int tc358746_format_supported(u32 code)
> > +{
> > + unsigned int i;
> > +
> > + for (i = 0; i < ARRAY_SIZE(tc358746_formats); i++)
> > + if (tc358746_formats[i].code == code)
> > + return 0;
> > +
> > + return -EINVAL;
> > +}
> > +
> > +static struct tc358746_csi_param *
> > +tc358746_g_cur_csi_settings(struct tc358746_state *state)
> > +{
> > + unsigned int cur_freq = v4l2_ctrl_g_ctrl(state->link_freq);
> > +
> > + return &state->link_freq_settings[cur_freq];
> > +}
> > +
> > +static const struct tc358746_mbus_fmt *tc358746_get_format(u32 code)
> > +{
> > + unsigned int i;
> > +
> > + for (i = 0; i < ARRAY_SIZE(tc358746_formats); i++)
> > + if (tc358746_formats[i].code == code)
> > + return &tc358746_formats[i];
> > +
> > + return NULL;
> > +}
> > +
> > +static int
> > +tc358746_adjust_fifo_size(struct tc358746_state *state,
> > + const struct tc358746_mbus_fmt *format,
> > + struct tc358746_csi_param *csi_settings,
> > + unsigned int width, u16 *fifo_size)
> > +{
> > + int c_hactive_ps_diff, c_lp_active_ps_diff, c_fifo_delay_ps_diff;
> > + unsigned int p_hactive_ps, p_hblank_ps, p_htotal_ps;
> > + unsigned int c_hactive_ps, c_lp_active_ps, c_fifo_delay_ps;
> > + unsigned int csi_bps, csi_bps_period_ps;
> > + unsigned int csi_hsclk, csi_hsclk_period_ps;
> > + unsigned int pclk_period_ps;
> > + unsigned int _fifo_size;
> > +
> > + pclk_period_ps = 1000000000 / (state->pclk / 1000);
> > + csi_bps = csi_settings->speed_per_lane * csi_settings->lane_num;
> > + csi_bps_period_ps = 1000000000 / (csi_bps / 1000);
> > + csi_hsclk = csi_settings->speed_per_lane >> 3;
> > + csi_hsclk_period_ps = 1000000000 / (csi_hsclk / 1000);
> > +
> > + /*
> > + * Calculation:
> > + * p_hactive_ps = pclk_period_ps * pclk_per_pixel * h_active_pixel
> > + */
> > + p_hactive_ps = pclk_period_ps * format->ppp * width;
> > +
> > + /*
> > + * Calculation:
> > + * p_hblank_ps = pclk_period_ps * h_blank_pixel
> > + */
> > + p_hblank_ps = pclk_period_ps * state->hblank;
> > + p_htotal_ps = p_hblank_ps + p_hactive_ps;
> > +
> > + /*
> > + * Adjust the fifo size to adjust the csi timing. Hopefully we can find
> > + * a fifo size where the parallel input timings and the csi tx timings
> > + * fit together.
> > + */
> > + for (_fifo_size = 1; _fifo_size < TC358746_MAX_FIFO_SIZE;
> > + _fifo_size++) {
> > + /*
> > + * Calculation:
> > + * c_fifo_delay_ps = (fifo_size * 32) / parallel_bus_width *
> > + * pclk_period_ps + 4 * csi_hsclk_period_ps
> > + */
> > + c_fifo_delay_ps = _fifo_size * 32 * pclk_period_ps;
> > + c_fifo_delay_ps /= format->bus_width;
> > + c_fifo_delay_ps += 4 * csi_hsclk_period_ps;
> > +
> > + /*
> > + * Calculation:
> > + * c_hactive_ps = csi_bps_period_ps * image_bpp * h_active_pixel
> > + * + c_fifo_delay
> > + */
> > + c_hactive_ps = csi_bps_period_ps * format->bpp * width;
> > + c_hactive_ps += c_fifo_delay_ps;
> > +
> > + /*
> > + * Calculation:
> > + * c_lp_active_ps = p_htotal_ps - c_hactive_ps
> > + */
> > + c_lp_active_ps = p_htotal_ps - c_hactive_ps;
> > +
> > + c_hactive_ps_diff = c_hactive_ps - p_hactive_ps;
> > + c_fifo_delay_ps_diff = p_htotal_ps - c_hactive_ps;
> > + c_lp_active_ps_diff =
> > + c_lp_active_ps - csi_settings->csi_hs_lp_hs_ps;
> > +
> > + if (c_hactive_ps_diff > 0 &&
> > + c_fifo_delay_ps_diff > 0 &&
> > + c_lp_active_ps_diff > 0)
> > + break;
> > + }
> > +
> > + /*
> > + * If we can't transfer the image using this csi link frequency try to
> > + * use another link freq.
> > + */
> > + v4l2_dbg(1, debug, &state->sd, "found fifo-size %d\n",
> > + _fifo_size == TC358746_MAX_FIFO_SIZE ? -1 : _fifo_size);
> > + *fifo_size = _fifo_size;
> > + return _fifo_size == TC358746_MAX_FIFO_SIZE ? -EINVAL : 0;
> > +}
> > +
> > +static int
> > +tc358746_adjust_timings(struct tc358746_state *state,
> > + const struct tc358746_mbus_fmt *format,
> > + unsigned int *width, u16 *fifo_size)
> > +{
> > +
> > + unsigned int cur_freq = v4l2_ctrl_g_ctrl(state->link_freq);
> > + unsigned int freq = cur_freq;
> > + struct tc358746_csi_param *csi_lane_setting;
> > + int err;
> > + int _width;
> > +
> > + /*
> > + * Adjust timing:
> > + * 1) Try to use the desired width and the current csi-link-frequency
> > + * 2) If this doesn't fit try other csi-link-frequencies
> > + * 3) If this doesn't fit too, reducing the desired width and test
> > + * it again width the current csi-link-frequency
> > + * 4) Goto step 2 if it doesn't fit at all
> > + */
> > + for (_width = *width; _width > 0; _width -= 10) {
> > + csi_lane_setting = &state->link_freq_settings[cur_freq];
> > + err = tc358746_adjust_fifo_size(state, format, csi_lane_setting,
> > + _width, fifo_size);
> > + if (!err)
> > + goto out;
> > +
> > + for (freq = 0; freq < state->link_frequencies_num; freq++) {
> > + if (freq == cur_freq)
> > + continue;
> > +
> > + csi_lane_setting = &state->link_freq_settings[freq];
> > + err = tc358746_adjust_fifo_size(state, format,
> > + csi_lane_setting,
> > + _width, fifo_size);
> > + if (!err)
> > + goto out;
> > + }
> > + }
> > +
> > +out:
> > + *width = _width;
> > + return freq;
> > +}
> > +
> > +static int
> > +tc358746_calculate_csi_txtimings(struct tc358746_state *state,
> > + struct tc358746_csi_param *csi_setting)
> > +{
> > + unsigned int spl;
> > + unsigned int spl_p_ps, hsclk_p_ps, hfclk_p_ns;
> > + unsigned int hfclk, hsclk; /* SYSCLK */
> > + unsigned int tmp;
> > + unsigned int lptxtime_ps, tclk_post_ps, tclk_trail_ps, tclk_zero_ps,
> > + ths_trail_ps, ths_zero_ps;
> > +
> > + spl = csi_setting->speed_per_lane;
> > + hsclk = spl >> 3; /* spl in bit-per-second, hsclk in byte-per-sercond */
> > + hfclk = hsclk >> 1; /* HFCLK = SYSCLK / 2 */
> > +
> > + if (hsclk > 125000000U) {
> > + v4l2_err(&state->sd, "unsupported HS byte clock %d, must <= 125 MHz\n",
> > + hsclk);
> > + return -EINVAL;
> > + }
> > +
> > + hfclk_p_ns = DIV_ROUND_CLOSEST(1000000000, hfclk);
> > + hsclk_p_ps = 1000000000 / (hsclk / 1000);
> > + spl_p_ps = 1000000000 / (spl / 1000);
> > +
> > + /*
> > + * Calculation:
> > + * hfclk_p_ns * lineinitcnt > 100us
> > + * lineinitcnt > 100 * 10^-6s / hfclk_p_ns * 10^-9
> > + *
> > + */
> > + csi_setting->lineinitcnt = DIV_ROUND_UP(TC358746_LINEINIT_MIN_US * 1000,
> > + hfclk_p_ns);
> > +
> > + /*
> > + * Calculation:
> > + * (lptxtimecnt + 1) * hsclk_p_ps > 50ns
> > + * 38ns < (tclk_preparecnt + 1) * hsclk_p_ps < 95ns
> > + */
> > + csi_setting->lptxtimecnt = csi_setting->tclk_preparecnt =
> > + DIV_ROUND_UP(TC358746_LPTXTIME_MIN_NS * 1000, hsclk_p_ps) - 1;
> > +
> > + /*
> > + * Limit:
> > + * (tclk_zero + tclk_prepar) period > 300ns.
> > + * Since we have no upper limit and for simplicity:
> > + * tclk_zero > 300ns.
> > + *
> > + * Calculation:
> > + * tclk_zero = ([2,3] + tclk_zerocnt) * hsclk_p_ps + ([2,3] * spl_p_ps)
> > + *
> > + * Note: REF_02 uses
> > + * tclk_zero = (2.5 + tclk_zerocnt) * hsclk_p_ps + (3.5 * spl_p_ps)
> > + */
> > + tmp = TC358746_TCLKZERO_MIN_NS * 1000 - 3 * spl_p_ps;
> > + tmp = DIV_ROUND_UP(tmp, hsclk_p_ps);
> > + csi_setting->tclk_zerocnt = tmp - 2;
> > +
> > + /*
> > + * Limit:
> > + * 40ns + 4 * spl_p_ps < (ths_preparecnt + 1) * hsclk_p_ps
> > + * < 85ns + 6 * spl_p_ps
> > + */
> > + tmp = TC358746_THSPREPARE_MIN_NS * 1000 + 4 * spl_p_ps;
> > + tmp = DIV_ROUND_UP(tmp, hsclk_p_ps);
> > + csi_setting->ths_preparecnt = tmp - 1;
> > +
> > + /*
> > + * Limit:
> > + * (ths_zero + ths_prepare) period > 145ns + 10 * spl_p_ps.
> > + * Since we have no upper limit and for simplicity:
> > + * ths_zero period > 145ns + 10 * spl_p_ps.
> > + *
> > + * Calculation:
> > + * ths_zero = ([6,8] + ths_zerocnt) * hsclk_p_ps + [3,4] * hsclk_p_ps +
> > + * [13,14] * spl_p_ps
> > + *
> > + * Note: REF_02 uses
> > + * ths_zero = (7 + ths_zerocnt) * hsclk_p_ps + 4 * hsclk_p_ps +
> > + * 11 * spl_p_ps
> > + */
> > + tmp = TC358746_THSZERO_MIN_NS * 1000 - spl_p_ps;
> > + tmp = DIV_ROUND_UP(tmp, hsclk_p_ps);
> > + csi_setting->ths_zerocnt = tmp < 11 ? 0 : tmp - 11;
> > +
> > + /*
> > + * Limit:
> > + * hsclk_p_ps * (lptxtimecnt + 1) * (twakeupcnt + 1) > 1ms
> > + *
> > + * Since we have no upper limit use 1.2ms as lower limit to
> > + * surley meet the spec limit.
> > + */
> > + tmp = hsclk_p_ps / 1000; /* tmp = hsclk_p_ns */
> > + csi_setting->twakeupcnt =
> > + DIV_ROUND_UP(TC358746_TWAKEUP_MIN_US * 1000,
> > + tmp * (csi_setting->lptxtimecnt + 1)) - 1;
> > +
> > + /*
> > + * Limit:
> > + * 60ns + 4 * spl_p_ps < thstrail < 105ns + 12 * spl_p_ps
> > + *
> > + * Calculation:
> > + * thstrail = (1 + ths_trailcnt) * hsclk_p_ps + [3,4] * hsclk_p_ps -
> > + * [13,14] * spl_p_ps
> > + *
> > + * [2] set formula to:
> > + * thstrail = (1 + ths_trailcnt) * hsclk_p_ps + 4 * hsclk_p_ps -
> > + * 11 * spl_p_ps
> > + */
> > + tmp = TC358746_THSTRAIL_MIN_NS * 1000 + 15 * spl_p_ps;
> > + tmp = DIV_ROUND_UP(tmp, hsclk_p_ps);
> > + csi_setting->ths_trailcnt = tmp - 5;
> > +
> > + /*
> > + * Limit:
> > + * 60ns < tclk_trail < 105ns + 12 * spl_p_ps
> > + *
> > + * Limit used by REF_02:
> > + * 60ns < tclk_trail < 105ns + 12 * spl_p_ps - 30
> > + *
> > + * Calculation:
> > + * tclk_trail = ([1,2] + tclk_trailcnt) * hsclk_p_ps +
> > + * (2 + [1,2]) * hsclk_p_ps - [2,3] * spl_p_ps
> > + *
> > + * Calculation used by REF_02:
> > + * tclk_trail = (1 + tclk_trailcnt) * hsclk_p_ps +
> > + * 4 * hsclk_p_ps - 3 * spl_p_ps
> > + */
> > + tmp = TC358746_TCLKTRAIL_MIN_NS * 1000 + 3 * spl_p_ps;
> > + tmp = DIV_ROUND_UP(tmp, hsclk_p_ps);
> > + csi_setting->tclk_trailcnt = tmp < 5 ? 0 : tmp - 5;
> > +
> > + /*
> > + * Limit:
> > + * tclk_post > 60ns + 52 * spl_p_ps
> > + *
> > + * Limit used by REF_02:
> > + * tclk_post > 60ns + 52 * spl_p_ps
> > + *
> > + * Calculation:
> > + * tclk_post = ([1,2] + (tclk_postcnt + 1)) * hsclk_p_ps + hsclk_p_ps
> > + *
> > + * Note REF_02 uses:
> > + * tclk_post = (2.5 + tclk_postcnt) * hsclk_p_ps + hsclk_p_ps +
> > + * 2.5 * spl_p_ps
> > + * To meet the REF_02 validation limits following equation is used:
> > + * tclk_post = (2 + tclk_postcnt) * hsclk_p_ps + hsclk_p_ps +
> > + * 3 * spl_p_ps
> > + */
> > + tmp = TC358746_TCLKPOST_MIN_NS * 1000 + 49 * spl_p_ps;
> > + tmp = DIV_ROUND_UP(tmp, hsclk_p_ps);
> > + csi_setting->tclk_postcnt = tmp - 3;
> > +
> > + /*
> > + * Last calculate the csi hs->lp->hs transistion time in ns. Note REF_02
> > + * mixed units in the equation for the continuous case. I don't know if
> > + * this was the intention. The driver drops the last 'multiply all by
> > + * two' to get nearly the same results.
> > + */
> > + lptxtime_ps = (csi_setting->lptxtimecnt + 1) * hsclk_p_ps;
> > + tclk_post_ps =
> > + (4 + csi_setting->tclk_postcnt) * hsclk_p_ps + 3 * spl_p_ps;
> > + tclk_trail_ps =
> > + (5 + csi_setting->tclk_trailcnt) * hsclk_p_ps - 3 * spl_p_ps;
> > + tclk_zero_ps =
> > + (2 + csi_setting->tclk_zerocnt) * hsclk_p_ps + 3 * spl_p_ps;
> > + ths_trail_ps =
> > + (5 + csi_setting->ths_trailcnt) * hsclk_p_ps - 11 * spl_p_ps;
> > + ths_zero_ps =
> > + (7 + csi_setting->ths_zerocnt) * hsclk_p_ps + 4 * hsclk_p_ps +
> > + 11 * spl_p_ps;
> > +
> > + if (csi_setting->is_continuous_clk) {
> > + tmp = 2 * lptxtime_ps;
> > + tmp += 25 * hsclk_p_ps;
> > + tmp += ths_trail_ps;
> > + tmp += ths_zero_ps;
> > + } else {
> > + tmp = 4 * lptxtime_ps;
> > + tmp += ths_trail_ps + tclk_post_ps + tclk_trail_ps +
> > + tclk_zero_ps + ths_zero_ps;
> > + tmp += (13 + csi_setting->lptxtimecnt * 8) * hsclk_p_ps;
> > + tmp += 22 * hsclk_p_ps;
> > + tmp *= 3;
> > + tmp = DIV_ROUND_CLOSEST(tmp, 2);
> > + }
> > + csi_setting->csi_hs_lp_hs_ps = tmp;
> > +
> > + return 0;
> > +}
> > +
> > +/* --------------- i2c helper ------------ */
> > +
> > +static void i2c_rd(struct v4l2_subdev *sd, u16 reg, u8 *values, u32 n)
> > +{
> > + struct tc358746_state *state = to_state(sd);
> > + struct i2c_client *client = state->i2c_client;
> > + int err;
> > + u8 buf[2] = { reg >> 8, reg & 0xff };
> > + u8 data[I2C_MAX_XFER_SIZE];
> > +
> > + struct i2c_msg msgs[] = {
> > + {
> > + .addr = client->addr,
> > + .flags = 0,
> > + .len = 2,
> > + .buf = buf,
> > + },
> > + {
> > + .addr = client->addr,
> > + .flags = I2C_M_RD,
> > + .len = n,
> > + .buf = data,
> > + },
> > + };
> > +
> > + err = i2c_transfer(client->adapter, msgs, ARRAY_SIZE(msgs));
> > + if (err != ARRAY_SIZE(msgs)) {
> > + v4l2_err(sd, "%s: reading register 0x%x from 0x%x failed\n",
> > + __func__, reg, client->addr);
> > + }
> > +
> > + switch (n) {
> > + case 1:
> > + values[0] = data[0];
> > + break;
> > + case 2:
> > + values[0] = data[1];
> > + values[1] = data[0];
> > + break;
> > + case 4:
> > + values[0] = data[1];
> > + values[1] = data[0];
> > + values[2] = data[3];
> > + values[3] = data[2];
> > + break;
> > + default:
> > + v4l2_info(sd, "unsupported I2C read %d bytes from address 0x%04x\n",
> > + n, reg);
> > + }
> > +
> > + if (debug < 3)
> > + return;
> > +
> > + switch (n) {
> > + case 1:
> > + v4l2_info(sd, "I2C read 0x%04x = 0x%02x",
> > + reg, data[0]);
> > + break;
> > + case 2:
> > + v4l2_info(sd, "I2C read 0x%04x = 0x%02x%02x",
> > + reg, data[0], data[1]);
> > + break;
> > + case 4:
> > + v4l2_info(sd, "I2C read 0x%04x = 0x%02x%02x%02x%02x",
> > + reg, data[2], data[3], data[0], data[1]);
> > + break;
> > + default:
> > + v4l2_info(sd, "I2C unsupported read %d bytes from address 0x%04x\n",
> > + n, reg);
> > + }
> > +}
> > +
> > +static void i2c_wr(struct v4l2_subdev *sd, u16 reg, u8 *values, u32 n)
> > +{
> > + struct tc358746_state *state = to_state(sd);
> > + struct i2c_client *client = state->i2c_client;
> > + int err;
> > + struct i2c_msg msg;
> > + u8 data[I2C_MAX_XFER_SIZE];
> > +
> > + if ((2 + n) > I2C_MAX_XFER_SIZE) {
> > + n = I2C_MAX_XFER_SIZE - 2;
> > + v4l2_warn(sd, "i2c wr reg=%04x: len=%d is too big!\n",
> > + reg, 2 + n);
> > + }
> > +
> > + msg.addr = client->addr;
> > + msg.buf = data;
> > + msg.len = 2 + n;
> > + msg.flags = 0;
> > +
> > + data[0] = reg >> 8;
> > + data[1] = reg & 0xff;
> > +
> > + switch (n) {
> > + case 1:
> > + data[2 + 0] = values[0];
> > + break;
> > + case 2:
> > + data[2 + 0] = values[1];
> > + data[2 + 1] = values[0];
> > + break;
> > + case 4:
> > + data[2 + 0] = values[1];
> > + data[2 + 1] = values[0];
> > + data[2 + 2] = values[3];
> > + data[2 + 3] = values[2];
> > + break;
> > + default:
> > + v4l2_info(sd, "unsupported I2C write %d bytes from address 0x%04x\n",
> > + n, reg);
>
> You'd need to return here. Perhaps WARN_ON(1) instead of v4l2_info(), as
> this would presumably be a driver bug.
Okay.
> > + }
> > +
> > + err = i2c_transfer(client->adapter, &msg, 1);
> > + if (err != 1) {
> > + v4l2_err(sd, "%s: writing register 0x%x from 0x%x failed\n",
> > + __func__, reg, client->addr);
> > + return;
> > + }
> > +
> > + if (debug < 3)
> > + return;
> > +
> > + switch (n) {
> > + case 1:
> > + v4l2_info(sd, "I2C write 0x%04x = 0x%02x", reg, data[2 + 0]);
> > + break;
> > + case 2:
> > + v4l2_info(sd, "I2C write 0x%04x = 0x%02x%02x", reg, data[2 + 0],
> > + data[2 + 1]);
> > + break;
> > + case 4:
> > + v4l2_info(sd, "I2C write 0x%04x = 0x%02x%02x%02x%02x", reg,
> > + data[2 + 2], data[2 + 3], data[2 + 0], data[2 + 1]);
> > + break;
> > + default:
> > + v4l2_info(sd, "I2C unsupported write %d bytes from address 0x%04x\n",
> > + n, reg);
> > + }
> > +}
> > +
> > +static noinline u32 i2c_rdreg(struct v4l2_subdev *sd, u16 reg, u32 n)
> > +{
> > + __le32 val = 0;
> > +
> > + i2c_rd(sd, reg, (u8 __force *)&val, n);
> > +
> > + return le32_to_cpu(val);
> > +}
> > +
> > +static noinline void i2c_wrreg(struct v4l2_subdev *sd, u16 reg, u32 val, u32 n)
> > +{
> > + __le32 raw = cpu_to_le32(val);
> > +
> > + i2c_wr(sd, reg, (u8 __force *)&raw, n);
> > +}
> > +
> > +static u16 __maybe_unused i2c_rd8(struct v4l2_subdev *sd, u16 reg)
> > +{
> > + return i2c_rdreg(sd, reg, 1);
> > +}
> > +
> > +static u16 __maybe_unused i2c_rd16(struct v4l2_subdev *sd, u16 reg)
> > +{
> > + return i2c_rdreg(sd, reg, 2);
> > +}
> > +
> > +static u32 __maybe_unused i2c_rd32(struct v4l2_subdev *sd, u16 reg)
> > +{
> > + return i2c_rdreg(sd, reg, 4);
> > +}
> > +
> > +static void __maybe_unused i2c_wr8(struct v4l2_subdev *sd, u16 reg, u16 val)
> > +{
> > + i2c_wrreg(sd, reg, val, 1);
> > +}
> > +
> > +static void i2c_wr16(struct v4l2_subdev *sd, u16 reg, u16 val)
> > +{
> > + i2c_wrreg(sd, reg, val, 2);
> > +}
> > +
> > +static void i2c_wr16_and_or(struct v4l2_subdev *sd, u16 reg, u32 mask, u16 val)
> > +{
> > + u16 m = (u16) ~mask;
> > +
> > + i2c_wrreg(sd, reg, (i2c_rd16(sd, reg) & m) | val, 2);
> > +}
> > +
> > +static void i2c_wr32(struct v4l2_subdev *sd, u16 reg, u32 val)
> > +{
> > + i2c_wrreg(sd, reg, val, 4);
> > +}
> > +
> > +/* --------------- init --------------- */
> > +
> > +static void
> > +tc358746_wr_csi_control(struct v4l2_subdev *sd, int val)
> > +{
> > + u32 _val;
> > +
> > + val &= CSI_CONFW_DATA_MASK;
> > + _val = CSI_CONFW_MODE_SET_MASK | CSI_CONFW_ADDRESS_CSI_CONTROL_MASK |
> > + val;
> > +
> > + v4l2_dbg(2, debug, sd, "CSI_CONFW 0x%04x\n", _val);
> > + i2c_wr32(sd, CSI_CONFW, _val);
> > +}
> > +
> > +static inline void tc358746_sleep_mode(struct v4l2_subdev *sd, int enable)
> > +{
> > + i2c_wr16_and_or(sd, SYSCTL, SYSCTL_SLEEP_MASK,
> > + enable ? SYSCTL_SLEEP_MASK : 0);
> > +}
> > +
> > +static inline void tc358746_sreset(struct v4l2_subdev *sd)
> > +{
> > + i2c_wr16(sd, SYSCTL, SYSCTL_SRESET_MASK);
> > + udelay(10);
> > + i2c_wr16(sd, SYSCTL, 0);
> > +}
> > +
> > +static inline void tc358746_enable_stream(struct v4l2_subdev *sd, int enable)
> > +{
> > + struct tc358746_state *state = to_state(sd);
> > +
> > + v4l2_dbg(1, debug, sd, "%sable\n", enable ? "en" : "dis");
> > +
> > + mutex_lock(&state->confctl_mutex);
> > + if (!enable) {
> > + i2c_wr16_and_or(sd, PP_MISC, PP_MISC_FRMSTOP_MASK,
> > + PP_MISC_FRMSTOP_MASK);
> > + i2c_wr16_and_or(sd, CONFCTL, CONFCTL_PPEN_MASK, 0);
> > + i2c_wr16_and_or(sd, PP_MISC, PP_MISC_RSTPTR_MASK,
> > + PP_MISC_RSTPTR_MASK);
> > +
> > + i2c_wr32(sd, CSIRESET, (CSIRESET_RESET_CNF_MASK |
> > + CSIRESET_RESET_MODULE_MASK));
> > + i2c_wr16(sd, DBG_ACT_LINE_CNT, 0);
> > + } else {
> > + i2c_wr16(sd, PP_MISC, 0);
> > + i2c_wr16_and_or(sd, CONFCTL, CONFCTL_PPEN_MASK,
> > + CONFCTL_PPEN_MASK);
> > + }
> > + mutex_unlock(&state->confctl_mutex);
> > +}
> > +
> > +static void tc358746_set_pll(struct v4l2_subdev *sd)
> > +{
> > + struct tc358746_state *state = to_state(sd);
> > + struct tc358746_csi_param *csi_setting =
> > + tc358746_g_cur_csi_settings(state);
> > + u16 pllctl0 = i2c_rd16(sd, PLLCTL0);
> > + u16 pllctl1 = i2c_rd16(sd, PLLCTL1);
> > + u16 pll_frs = csi_setting->speed_range;
> > + u16 pllctl0_new;
> > +
> > + /*
> > + * Calculation:
> > + * speed_per_lane = (pllinclk_hz * (fbd + 1)) / 2^frs
> > + *
> > + * Calculation used by REF_02:
> > + * speed_per_lane = (pllinclk_hz * fbd) / 2^frs
> > + */
> > + state->pll_fbd = csi_setting->speed_per_lane / state->pllinclk_hz;
> > + state->pll_fbd <<= pll_frs;
> > +
> > + pllctl0_new = PLLCTL0_PLL_PRD_SET(state->pll_prd) |
> > + PLLCTL0_PLL_FBD_SET(state->pll_fbd);
> > +
> > + /*
> > + * Only rewrite when needed (new value or disabled), since rewriting
> > + * triggers another format change event.
> > + */
> > + if ((pllctl0 != pllctl0_new) ||
> > + ((pllctl1 & PLLCTL1_PLL_EN_MASK) == 0)) {
> > + u16 pllctl1_mask = PLLCTL1_PLL_FRS_MASK | PLLCTL1_RESETB_MASK |
> > + PLLCTL1_PLL_EN_MASK;
> > + u16 pllctl1_val = PLLCTL1_PLL_FRS_SET(pll_frs) |
> > + PLLCTL1_RESETB_MASK | PLLCTL1_PLL_EN_MASK;
> > +
> > + v4l2_dbg(1, debug, sd, "Updating PLL clock\n");
> > + i2c_wr16(sd, PLLCTL0, pllctl0_new);
> > + i2c_wr16_and_or(sd, PLLCTL1, pllctl1_mask, pllctl1_val);
> > + udelay(1000);
> > + i2c_wr16_and_or(sd, PLLCTL1, PLLCTL1_CKEN_MASK,
> > + PLLCTL1_CKEN_MASK);
> > + }
> > +
> > + tc358746_dump_pll(sd, state);
> > +}
> > +
> > +static void tc358746_set_csi_color_space(struct v4l2_subdev *sd)
> > +{
> > + struct tc358746_state *state = to_state(sd);
> > + const struct tc358746_mbus_fmt *tc358746_fmt =
> > + tc358746_get_format(state->fmt.code);
> > +
> > + /* currently no self defined csi user data type id's are supported */
> > + mutex_lock(&state->confctl_mutex);
> > + i2c_wr16_and_or(sd, DATAFMT,
> > + (DATAFMT_PDFMT_MASK | DATAFMT_UDT_EN_MASK),
> > + DATAFMT_PDFMT_SET(tc358746_fmt->pdformat));
> > + i2c_wr16_and_or(sd, CONFCTL, CONFCTL_PDATAF_MASK,
> > + CONFCTL_PDATAF_SET(tc358746_fmt->pdataf));
> > + mutex_unlock(&state->confctl_mutex);
> > +}
> > +
> > +static void tc38764_debug_pattern_80(struct v4l2_subdev *sd)
> > +{
> > + unsigned int i;
> > +
> > + i2c_wr16(sd, DBG_ACT_LINE_CNT, 0x8000);
> > + i2c_wr16(sd, DBG_LINE_WIDTH, 0x0396);
> > + i2c_wr16(sd, DBG_VERT_BLANK_LINE_CNT, 0x0000);
> > +
> > + for (i = 0; i < 80; i++)
> > + i2c_wr16(sd, DBG_VIDEO_DATA, 0xff7f);
> > + i2c_wr16(sd, DBG_VIDEO_DATA, 0xff00);
> > + for (i = 0; i < 40; i++)
> > + i2c_wr16(sd, DBG_VIDEO_DATA, 0xffff);
> > + i2c_wr16(sd, DBG_VIDEO_DATA, 0xc0ff);
> > + for (i = 0; i < 40; i++)
> > + i2c_wr16(sd, DBG_VIDEO_DATA, 0xc000);
> > + for (i = 0; i < 80; i++)
> > + i2c_wr16(sd, DBG_VIDEO_DATA, 0x7f00);
> > + for (i = 0; i < 80; i++)
> > + i2c_wr16(sd, DBG_VIDEO_DATA, 0x7fff);
> > + i2c_wr16(sd, DBG_VIDEO_DATA, 0x0000);
> > + for (i = 0; i < 40; i++)
> > + i2c_wr16(sd, DBG_VIDEO_DATA, 0x00ff);
> > + i2c_wr16(sd, DBG_VIDEO_DATA, 0x00ff);
> > + for (i = 0; i < 40; i++)
> > + i2c_wr16(sd, DBG_VIDEO_DATA, 0x0000);
> > + i2c_wr16(sd, DBG_VIDEO_DATA, 0x007f);
> > +
> > + i2c_wr16(sd, DBG_ACT_LINE_CNT, 0xC1DF);
> > +}
> > +
> > +static void tc358746_enable_csi_lanes(struct v4l2_subdev *sd, int enable)
> > +{
> > + struct tc358746_state *state = to_state(sd);
> > + struct tc358746_csi_param *csi_setting =
> > + tc358746_g_cur_csi_settings(state);
> > + unsigned int lanes = csi_setting->lane_num;
> > + u32 val = 0;
> > +
> > + if (lanes < 1 || !enable)
> > + i2c_wr32(sd, CLW_CNTRL, CLW_CNTRL_CLW_LANEDISABLE_MASK);
> > + if (lanes < 1 || !enable)
> > + i2c_wr32(sd, D0W_CNTRL, D0W_CNTRL_D0W_LANEDISABLE_MASK);
>
> Please put the two in the same basic block; no need for two conditions that
> are the same.
>
> > + if (lanes < 2 || !enable)
> > + i2c_wr32(sd, D1W_CNTRL, D1W_CNTRL_D1W_LANEDISABLE_MASK);
> > + if (lanes < 3 || !enable)
> > + i2c_wr32(sd, D2W_CNTRL, D2W_CNTRL_D2W_LANEDISABLE_MASK);
> > + if (lanes < 4 || !enable)
> > + i2c_wr32(sd, D3W_CNTRL, D2W_CNTRL_D3W_LANEDISABLE_MASK);
> > +
> > + if (lanes > 0 && enable)
>
> Isn't lanes always greater than 0?
Sure.. changed this and above comment.
> > + val |= HSTXVREGEN_CLM_HSTXVREGEN_MASK |
> > + HSTXVREGEN_D0M_HSTXVREGEN_MASK;
> > + if (lanes > 1 && enable)
> > + val |= HSTXVREGEN_D1M_HSTXVREGEN_MASK;
> > + if (lanes > 2 && enable)
> > + val |= HSTXVREGEN_D2M_HSTXVREGEN_MASK;
> > + if (lanes > 3 && enable)
> > + val |= HSTXVREGEN_D3M_HSTXVREGEN_MASK;
> > +
> > + i2c_wr32(sd, HSTXVREGEN, val);
> > +}
> > +
> > +static void tc358746_set_csi(struct v4l2_subdev *sd)
> > +{
> > + struct tc358746_state *state = to_state(sd);
> > + struct tc358746_csi_param *csi_setting =
> > + tc358746_g_cur_csi_settings(state);
> > + bool en_continuous_clk = csi_setting->is_continuous_clk;
> > + u32 val;
> > +
> > + val = TCLK_HEADERCNT_TCLK_ZEROCNT_SET(csi_setting->tclk_zerocnt) |
> > + TCLK_HEADERCNT_TCLK_PREPARECNT_SET(csi_setting->tclk_preparecnt);
> > + i2c_wr32(sd, TCLK_HEADERCNT, val);
> > + val = THS_HEADERCNT_THS_ZEROCNT_SET(csi_setting->ths_zerocnt) |
> > + THS_HEADERCNT_THS_PREPARECNT_SET(csi_setting->ths_preparecnt);
> > + i2c_wr32(sd, THS_HEADERCNT, val);
> > + i2c_wr32(sd, TWAKEUP, csi_setting->twakeupcnt);
> > + i2c_wr32(sd, TCLK_POSTCNT, csi_setting->tclk_postcnt);
> > + i2c_wr32(sd, THS_TRAILCNT, csi_setting->ths_trailcnt);
> > + i2c_wr32(sd, LINEINITCNT, csi_setting->lineinitcnt);
> > + i2c_wr32(sd, LPTXTIMECNT, csi_setting->lptxtimecnt);
> > + i2c_wr32(sd, TCLK_TRAILCNT, csi_setting->tclk_trailcnt);
> > + i2c_wr32(sd, TXOPTIONCNTRL,
> > + en_continuous_clk ? TXOPTIONCNTRL_CONTCLKMODE_MASK : 0);
> > +
> > + if (state->test)
> > + tc38764_debug_pattern_80(sd);
> > +
> > + tc358746_dump_csi(sd, csi_setting);
> > +}
> > +
> > +static void tc358746_enable_csi_module(struct v4l2_subdev *sd, int enable)
> > +{
> > + struct tc358746_state *state = to_state(sd);
> > + struct tc358746_csi_param *csi_setting =
> > + tc358746_g_cur_csi_settings(state);
> > + unsigned int lanes = csi_setting->lane_num;
> > + u32 val;
> > +
> > + if (!enable)
> > + return;
> > +
> > + i2c_wr32(sd, STARTCNTRL, STARTCNTRL_START_MASK);
> > + i2c_wr32(sd, CSI_START, CSI_START_STRT_MASK);
> > +
> > + val = CSI_CONTROL_NOL_1_MASK;
> > + if (lanes == 2)
> > + val = CSI_CONTROL_NOL_2_MASK;
> > + else if (lanes == 3)
> > + val = CSI_CONTROL_NOL_3_MASK;
> > + else if (lanes == 4)
> > + val = CSI_CONTROL_NOL_4_MASK;
> > +
> > + val |= CSI_CONTROL_CSI_MODE_MASK | CSI_CONTROL_TXHSMD_MASK;
> > + tc358746_wr_csi_control(sd, val);
> > +}
> > +
> > +static void tc358746_set_buffers(struct v4l2_subdev *sd)
> > +{
> > + struct tc358746_state *state = to_state(sd);
> > + const struct tc358746_mbus_fmt *tc358746_mbusfmt =
> > + tc358746_get_format(state->fmt.code);
> > + unsigned int byte_per_line =
> > + (state->fmt.width * tc358746_mbusfmt->bpp) / 8;
> > +
> > + i2c_wr16(sd, FIFOCTL, state->vb_fifo);
> > + i2c_wr16(sd, WORDCNT, byte_per_line);
> > + v4l2_dbg(1, debug, sd, "FIFOCTL 0x%02x: WORDCNT 0x%02x\n",
> > + state->vb_fifo, byte_per_line);
> > +}
> > +
> > +/* --------------- CORE OPS --------------- */
> > +
> > +static int tc358746_log_status(struct v4l2_subdev *sd)
> > +{
> > + struct tc358746_state *state = to_state(sd);
> > + uint16_t sysctl = i2c_rd16(sd, SYSCTL);
> > +
> > + v4l2_info(sd, "-----Chip status-----\n");
> > + v4l2_info(sd, "Chip ID: 0x%02lx\n",
> > + (i2c_rd16(sd, CHIPID) & CHIPID_CHIPID_MASK) >> 8);
> > + v4l2_info(sd, "Chip revision: 0x%02lx\n",
> > + i2c_rd16(sd, CHIPID) & CHIPID_REVID_MASK);
> > + v4l2_info(sd, "Sleep mode: %s\n", sysctl & SYSCTL_SLEEP_MASK ?
> > + "on" : "off");
> > +
> > + v4l2_info(sd, "-----CSI-TX status-----\n");
> > + v4l2_info(sd, "Waiting for particular sync signal: %s\n",
> > + (i2c_rd16(sd, CSI_STATUS) & CSI_STATUS_S_WSYNC_MASK) ?
> > + "yes" : "no");
> > + v4l2_info(sd, "Transmit mode: %s\n",
> > + (i2c_rd16(sd, CSI_STATUS) & CSI_STATUS_S_TXACT_MASK) ?
> > + "yes" : "no");
> > + v4l2_info(sd, "Stopped: %s\n",
> > + (i2c_rd16(sd, CSI_STATUS) & CSI_STATUS_S_HLT_MASK) ?
> > + "yes" : "no");
> > + v4l2_info(sd, "Color space: %s\n",
> > + state->fmt.code == MEDIA_BUS_FMT_UYVY8_2X8 ?
> > + "YCbCr 422 8-bit" : "Unsupported");
> > +
> > + return 0;
> > +}
> > +
> > +#ifdef CONFIG_VIDEO_ADV_DEBUG
> > +static void tc358746_print_register_map(struct v4l2_subdev *sd)
> > +{
> > + v4l2_info(sd, "0x0000-0x0050: Global Register\n");
> > + v4l2_info(sd, "0x0056-0x0070: Rx Control Registers\n");
> > + v4l2_info(sd, "0x0080-0x00F8: Rx Status Registers\n");
> > + v4l2_info(sd, "0x0100-0x0150: Tx D-PHY Register\n");
> > + v4l2_info(sd, "0x0204-0x0238: Tx PPI Register\n");
> > + v4l2_info(sd, "0x040c-0x0518: Tx Control Register\n");
> > +}
> > +
> > +static int tc358746_get_reg_size(u16 address)
> > +{
> > + if (address <= 0x00ff)
> > + return 2;
> > + else if ((address >= 0x0100) && (address <= 0x05FF))
> > + return 4;
> > + else
> > + return 1;
> > +}
> > +
> > +static int tc358746_g_register(struct v4l2_subdev *sd,
> > + struct v4l2_dbg_register *reg)
> > +{
> > + if (reg->reg > 0xffff) {
> > + tc358746_print_register_map(sd);
> > + return -EINVAL;
> > + }
> > +
> > + reg->size = tc358746_get_reg_size(reg->reg);
> > +
> > + reg->val = i2c_rdreg(sd, reg->reg, reg->size);
> > +
> > + return 0;
> > +}
> > +
> > +static int tc358746_s_register(struct v4l2_subdev *sd,
> > + const struct v4l2_dbg_register *reg)
> > +{
> > + if (reg->reg > 0xffff) {
> > + tc358746_print_register_map(sd);
> > + return -EINVAL;
> > + }
> > +
> > + i2c_wrreg(sd, (u16)reg->reg, reg->val,
> > + tc358746_get_reg_size(reg->reg));
> > +
> > + return 0;
> > +}
> > +#endif
> > +
> > +/* --------------- video ops --------------- */
> > +
> > +static int tc358746_s_power(struct v4l2_subdev *sd, int on)
> > +{
> > + struct tc358746_state *state = to_state(sd);
> > +
> > + /*
> > + * REF_01:
> > + * Softreset don't reset configuration registers content but is needed
>
> "doesn't"
>
> > + * during power-on to trigger a csi LP-11 state change and during
> > + * power-off to disable the csi-module.
> > + */
> > + tc358746_sreset(sd);
> > +
> > + if (state->fmt_changed) {
> > + tc358746_set_buffers(sd);
> > + tc358746_set_csi(sd);
> > + tc358746_set_csi_color_space(sd);
> > +
> > + /* as recommend in REF_01 */
> > + tc358746_sleep_mode(sd, 1);
> > + tc358746_set_pll(sd);
> > + tc358746_sleep_mode(sd, 0);
> > +
> > + state->fmt_changed = false;
> > + }
> > +
> > + tc358746_enable_csi_lanes(sd, on);
> > + tc358746_enable_csi_module(sd, on);
> > + tc358746_sleep_mode(sd, !on);
> > +
> > + return 0;
> > +}
> > +
> > +static int tc358746_s_stream(struct v4l2_subdev *sd, int enable)
> > +{
> > + tc358746_enable_stream(sd, enable);
>
> Could you use tc358746_enable_stream() directly as the s_stream op?
Of course, added it for naming consistency since the enable_stream() is
a internal function and I didn't wanted to mix those.
> Note that you need to call the upstream sub-device's s_stream op from here
> as well.
I tought this is done by the host driver trough iterating over the whole
graph?
> > +
> > + return 0;
> > +}
> > +
> > +/* --------------- pad ops --------------- */
> > +
> > +static int tc358746_enum_mbus_code(struct v4l2_subdev *sd,
> > + struct v4l2_subdev_pad_config *cfg,
> > + struct v4l2_subdev_mbus_code_enum *code)
> > +{
> > + if (code->index >= ARRAY_SIZE(tc358746_formats))
> > + return -EINVAL;
> > +
> > + code->code = tc358746_formats[code->index].code;
> > +
> > + return 0;
> > +}
> > +
> > +static struct v4l2_mbus_framefmt *
> > +__tc358746_get_pad_format(struct v4l2_subdev *sd,
> > + struct v4l2_subdev_pad_config *cfg,
> > + unsigned int pad, u32 which)
> > +{
> > + struct tc358746_state *state = to_state(sd);
> > +
> > + switch (which) {
> > + case V4L2_SUBDEV_FORMAT_TRY:
> > + return v4l2_subdev_get_try_format(sd, cfg, pad);
> > + case V4L2_SUBDEV_FORMAT_ACTIVE:
> > + return &state->fmt;
> > + default:
> > + return NULL;
> > + }
> > +}
> > +
> > +static int tc358746_get_fmt(struct v4l2_subdev *sd,
> > + struct v4l2_subdev_pad_config *cfg,
> > + struct v4l2_subdev_format *format)
> > +{
> > + struct tc358746_state *state = to_state(sd);
> > +
> > + if (format->pad != 0 && format->pad != 1)
> > + return -EINVAL;
> > +
> > + format->format.code = state->fmt.code;
> > + format->format.width = state->fmt.width;
> > + format->format.height = state->fmt.height;
> > + format->format.field = state->fmt.field;
> > +
> > + return 0;
> > +}
> > +
> > +static int tc358746_set_fmt(struct v4l2_subdev *sd,
> > + struct v4l2_subdev_pad_config *cfg,
> > + struct v4l2_subdev_format *format)
> > +{
> > + struct tc358746_state *state = to_state(sd);
> > + struct media_pad *pad = &state->pads[format->pad];
> > + struct v4l2_mbus_framefmt *mbusformat;
> > + const struct tc358746_mbus_fmt *tc358746_mbusformat;
> > + unsigned int pclk, hblank;
> > + unsigned int new_freq, cur_freq = v4l2_ctrl_g_ctrl(state->link_freq);
> > + u16 vb_fifo;
> > +
> > + if (pad->flags == MEDIA_PAD_FL_SOURCE)
> > + return tc358746_get_fmt(sd, cfg, format);
> > +
> > + mbusformat = __tc358746_get_pad_format(sd, cfg, format->pad,
> > + format->which);
> > + if (!mbusformat)
> > + return -EINVAL;
> > +
> > + tc358746_mbusformat = tc358746_get_format(format->format.code);
> > + if (!tc358746_mbusformat) {
> > + format->format.code = tc358746_def_fmt.code;
> > + tc358746_mbusformat = tc358746_get_format(format->format.code);
> > + }
> > +
> > + /*
> > + * Some sensors change their hblank and pclk value on different formats,
> > + * so we need to request it again.
> > + */
> > + pclk = v4l2_ctrl_g_ctrl_int64(state->sensor_pclk_ctrl);
> > + if (pclk != state->pclk) {
> > + v4l2_dbg(1, debug, sd, "Update pclk %u -> %u\n",
> > + state->pclk, pclk);
> > + state->pclk = pclk;
> > + }
> > +
> > + hblank = v4l2_ctrl_g_ctrl(state->sensor_hblank_ctrl);
> > + if (hblank != state->hblank) {
> > + v4l2_dbg(1, debug, sd, "Update hblank %u -> %u\n",
> > + state->hblank, hblank);
> > + state->hblank = hblank;
> > + }
> > +
> > + /*
> > + * Normaly the HW has no size limitations but we have to check if the
> > + * csi timings are valid for this size. The timings can be adjust by the
> > + * fifo size. If this doesn't work we have to do this check again with a
> > + * other csi link frequency if it is possible.
> > + */
> > + new_freq = tc358746_adjust_timings(state, tc358746_mbusformat,
> > + &format->format.width, &vb_fifo);
> > +
> > + /* Currently only a few YUV based formats are supported */
> > + if (tc358746_format_supported(format->format.code))
> > + format->format.code = MEDIA_BUS_FMT_UYVY8_2X8;
> > +
> > + /* Currently only non interleaved images are supported */
> > + format->format.field = V4L2_FIELD_NONE;
> > +
> > + *mbusformat = format->format;
> > +
> > + if (format->which == V4L2_SUBDEV_FORMAT_ACTIVE) {
> > + state->fmt_changed = true;
> > + state->vb_fifo = vb_fifo;
> > + if (new_freq != cur_freq)
> > + v4l2_ctrl_s_ctrl(state->link_freq, new_freq);
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int
> > +tc358746_link_validate(struct v4l2_subdev *sd, struct media_link *link,
> > + struct v4l2_subdev_format *source_fmt,
> > + struct v4l2_subdev_format *sink_fmt)
> > +{
> > + struct tc358746_state *state = to_state(sd);
> > + const struct tc358746_mbus_fmt *tc358746_mbusformat;
> > + unsigned int pclk, pclk_old = state->pclk;
> > + unsigned int hblank, hblank_old = state->hblank;
> > + int new_freq;
> > + u16 vb_fifo;
> > +
> > + /*
> > + * Only validate if the timings are changed, after the link was already
> > + * initialized. This can be happen if the parallel sensor frame interval
> > + * is changed. Format checks are perfomed by the common code.
> > + */
> > +
> > + tc358746_mbusformat = tc358746_get_format(sink_fmt->format.code);
> > + if (!tc358746_mbusformat)
> > + return -EINVAL; /* Format was changed too and is invalid */
> > +
> > + pclk = v4l2_ctrl_g_ctrl_int64(state->sensor_pclk_ctrl);
> > + if (pclk != state->pclk)
> > + state->pclk = pclk;
> > +
> > + hblank = v4l2_ctrl_g_ctrl(state->sensor_hblank_ctrl);
> > + if (hblank != state->hblank)
> > + state->hblank = hblank;
> > +
> > + new_freq = tc358746_adjust_timings(state, tc358746_mbusformat,
> > + &source_fmt->format.width, &vb_fifo);
> > +
> > + if (new_freq != v4l2_ctrl_g_ctrl(state->link_freq)) {
> > + /*
> > + * This can lead into undefined behaviour, so we don't support
> > + * dynamic changes due to a to late re-configuration.
> > + */
> > + state->pclk = pclk_old;
> > + state->hblank = hblank_old;
> > +
> > + return -EINVAL;
> > + }
> > +
> > + state->fmt_changed = true;
> > + state->vb_fifo = vb_fifo;
> > +
> > + return 0;
> > +}
> > +
> > +static int tc358764_s_ctrl(struct v4l2_ctrl *ctrl)
> > +{
> > + struct tc358746_state *state = container_of(ctrl->handler,
> > + struct tc358746_state, hdl);
> > + struct v4l2_subdev *sd = &state->sd;
> > +
> > + switch (ctrl->id) {
> > + case V4L2_CID_LINK_FREQ:
> > + v4l2_info(sd, "Update link-frequency %llu -> %llu\n",
> > + state->link_frequencies[ctrl->cur.val],
> > + state->link_frequencies[ctrl->val]);
> > +
> > + return 0;
> > + case V4L2_CID_TEST_PATTERN:
> > + state->test = ctrl->val;
> > + return 0;
> > + }
> > +
> > + return -EINVAL;
> > +}
> > +
> > +static int tc358746_link_setup(struct media_entity *entity,
> > + const struct media_pad *local,
> > + const struct media_pad *remote, u32 flags)
> > +{
> > + struct v4l2_subdev *sd = media_entity_to_v4l2_subdev(entity);
> > + struct v4l2_subdev *ps_sd = media_entity_to_v4l2_subdev(remote->entity);
> > + struct tc358746_state *state = to_state(sd);
> > + struct v4l2_ctrl *ctrl;
> > +
> > + /* no special requirements on source pads */
> > + if (local->flags & MEDIA_PAD_FL_SOURCE)
> > + return 0;
> > +
> > + v4l2_dbg(1, debug, sd, "link setup '%s':%d->'%s':%d[%d]",
> > + remote->entity->name, remote->index, local->entity->name,
> > + local->index, flags & MEDIA_LNK_FL_ENABLED);
> > +
> > + /*
> > + * The remote parallel sensor must support pixel rate and hblank query
> > + */
> > + ctrl = v4l2_ctrl_find(ps_sd->ctrl_handler, V4L2_CID_PIXEL_RATE);
> > + if (!ctrl) {
> > + v4l2_err(sd, "Subdev %s must support V4L2_CID_PIXEL_RATE\n",
> > + ps_sd->name);
> > + return -EINVAL;
> > + }
> > + state->pclk = v4l2_ctrl_g_ctrl_int64(ctrl);
> > + state->sensor_pclk_ctrl = ctrl;
> > +
> > + ctrl = v4l2_ctrl_find(ps_sd->ctrl_handler, V4L2_CID_HBLANK);
> > + if (!ctrl) {
> > + v4l2_err(sd, "Subdev %s must support V4L2_CID_HBLANK\n",
> > + ps_sd->name);
> > + return -EINVAL;
> > + }
> > + state->hblank = v4l2_ctrl_g_ctrl(ctrl);
> > + state->sensor_hblank_ctrl = ctrl;
> > +
> > + return 0;
> > +}
> > +
> > +/* -------------------------------------------------------------------------- */
> > +
> > +static const struct v4l2_ctrl_ops tc358764_ctrl_ops = {
> > + .s_ctrl = tc358764_s_ctrl,
> > +};
> > +
> > +static const struct v4l2_subdev_core_ops tc358746_core_ops = {
> > + .log_status = tc358746_log_status,
> > +#ifdef CONFIG_VIDEO_ADV_DEBUG
> > + .g_register = tc358746_g_register,
> > + .s_register = tc358746_s_register,
> > +#endif
> > + .s_power = tc358746_s_power,
> > +};
> > +
> > +static const struct v4l2_subdev_video_ops tc358746_video_ops = {
> > + .s_stream = tc358746_s_stream,
> > +};
> > +
> > +static const struct v4l2_subdev_pad_ops tc358746_pad_ops = {
> > + .enum_mbus_code = tc358746_enum_mbus_code,
> > + .set_fmt = tc358746_set_fmt,
> > + .get_fmt = tc358746_get_fmt,
> > + .link_validate = tc358746_link_validate,
> > +};
> > +
> > +static const struct v4l2_subdev_ops tc358746_ops = {
> > + .core = &tc358746_core_ops,
> > + .video = &tc358746_video_ops,
> > + .pad = &tc358746_pad_ops,
> > +};
> > +
> > +static const struct media_entity_operations tc358746_entity_ops = {
> > + .link_setup = &tc358746_link_setup,
> > + .link_validate = &v4l2_subdev_link_validate,
> > +};
> > +
> > +/* --------------- PROBE / REMOVE --------------- */
> > +
> > +static int tc358746_set_lane_settings(struct tc358746_state *state,
> > + struct v4l2_fwnode_endpoint *fw)
> > +{
> > + struct v4l2_subdev *sd = &state->sd;
> > + int i;
>
> unsigned int ?
>
> > +
> > + for (i = 0; i < fw->nr_of_link_frequencies; i++) {
> > + struct tc358746_csi_param *s =
> > + &state->link_freq_settings[i];
> > + u32 bps_pr_lane;
> > +
> > + state->link_frequencies[i] = fw->link_frequencies[i];
> > +
> > + /*
> > + * The CSI bps per lane must be between 62.5 Mbps and 1 Gbps.
> > + * bps_pr_lane = 2 * link_freq, because MIPI data lane is double
> > + * data rate.
> > + */
> > + bps_pr_lane = 2 * fw->link_frequencies[i];
> > + if (bps_pr_lane < 62500000U || bps_pr_lane > 1000000000U) {
> > + v4l2_err(sd, "unsupported bps per lane: %u bps\n",
> > + bps_pr_lane);
> > + return -EINVAL;
> > + }
> > +
> > + if (bps_pr_lane > 500000000)
> > + s->speed_range = 0;
> > + else if (bps_pr_lane > 250000000)
> > + s->speed_range = 1;
> > + else if (bps_pr_lane > 125000000)
> > + s->speed_range = 2;
> > + else
> > + s->speed_range = 3;
> > +
> > + s->unit_clk_hz = state->pllinclk_hz >> s->speed_range;
> > + s->unit_clk_mul = bps_pr_lane / s->unit_clk_hz;
> > + s->speed_per_lane = bps_pr_lane;
> > + s->lane_num = fw->bus.mipi_csi2.num_data_lanes;
> > + s->is_continuous_clk = fw->bus.mipi_csi2.flags &
> > + V4L2_MBUS_CSI2_CONTINUOUS_CLOCK;
> > +
> > + if (s->speed_per_lane != 432000000U)
> > + v4l2_warn(sd, "untested bps per lane: %u bps\n",
> > + s->speed_per_lane);
> > +
> > + v4l2_dbg(2, debug, sd, "lane setting %d\n", i);
> > + v4l2_dbg(2, debug, sd,
> > + "unit_clk %uHz: unit_clk_mul %u: speed_range %u: speed_per_lane(bps/lane) %u: csi_lange_numbers %u\n",
> > + s->unit_clk_hz, s->unit_clk_mul, s->speed_range,
> > + s->speed_per_lane, s->lane_num);
> > + }
> > +
> > + state->link_frequencies_num = fw->nr_of_link_frequencies;
> > +
> > + return 0;
> > +}
> > +
> > +static void tc358746_gpio_reset(struct tc358746_state *state)
> > +{
> > + usleep_range(5000, 10000);
> > + gpiod_set_value(state->reset_gpio, 1);
> > + usleep_range(1000, 2000);
> > + gpiod_set_value(state->reset_gpio, 0);
> > + msleep(20);
> > +}
> > +
> > +static int tc358746_apply_fw(struct tc358746_state *state)
> > +{
> > + struct v4l2_subdev *sd = &state->sd;
> > + struct tc358746_csi_param *csi_setting;
> > + int err, i;
>
> unsigned int i
Yes.
> > +
> > + for (i = 0; i < state->link_frequencies_num; i++) {
> > + csi_setting = &state->link_freq_settings[i];
> > +
> > + err = tc358746_calculate_csi_txtimings(state, csi_setting);
> > + if (err) {
> > + v4l2_err(sd, "Failed to calc csi-tx tminings\n");
> > + return err;
> > + }
> > + }
> > +
> > + /*
> > + * Set it to the hw default value. The correct value will be set during
> > + * set_fmt(), since it depends on the pclk and and the resulution.
> > + */
> > + state->vb_fifo = 1;
> > +
> > + err = clk_prepare_enable(state->refclk);
> > + if (err) {
> > + v4l2_err(sd, "Failed to enable clock\n");
> > + return err;
> > + }
>
> Is there a need to keep the clock running even if the device is not
> streaming? Please consider adding runtime PM support. That's not strictly
> needed for the patch to be merged though IMO.
Thats a good point. The Chip can act as clk-provider fo rother devices.
So I would probably say yes.
> > +
> > + if (state->reset_gpio)
> > + tc358746_gpio_reset(state);
> > +
> > + return 0;
> > +}
> > +
> > +static int tc358746_probe_fw(struct tc358746_state *state)
> > +{
> > + struct device *dev = &state->i2c_client->dev;
> > + struct v4l2_subdev *sd = &state->sd;
> > + struct v4l2_fwnode_endpoint endpoint = {
> > + .bus_type = V4L2_MBUS_CSI2_DPHY,
> > + };
> > + struct fwnode_handle *fw_node;
> > + unsigned int refclk, pllinclk;
> > + unsigned char pll_prediv;
> > + int ret = -EINVAL;
>
> ret is always assigned elsewhere; no need to initialise.
>
> > +
> > + /* Parse all clocks */
> > + state->refclk = devm_clk_get(dev, NULL);
> > + if (IS_ERR(state->refclk)) {
> > + if (PTR_ERR(state->refclk) != -EPROBE_DEFER)
> > + v4l2_err(sd, "failed to get refclk: %ld\n",
> > + PTR_ERR(state->refclk));
> > + return PTR_ERR(state->refclk);
> > + }
> > +
> > + refclk = clk_get_rate(state->refclk);
> > + if (refclk < 6000000 || refclk > 40000000) {
> > + v4l2_err(sd, "Invalid refclk %u, valid range: 6-40 MHz\n",
> > + refclk);
> > + return -EINVAL;
> > + }
> > +
> > + /*
> > + * The PLL input clock is obtained by dividing refclk by pll_prd.
> > + * It must be between 4 MHz and 40 MHz, lower frequency is better.
> > + */
> > + pll_prediv = DIV_ROUND_CLOSEST(refclk, 4000000);
> > + if (pll_prediv < 1 || pll_prediv > 16) {
> > + v4l2_err(sd, "Invalid pll pre-divider value: %d\n", pll_prediv);
> > + return -EINVAL;
> > + }
> > + state->pll_prd = pll_prediv;
> > +
> > + pllinclk = DIV_ROUND_CLOSEST(refclk, pll_prediv);
> > + if (pllinclk < 4000000 || pllinclk > 40000000) {
> > + v4l2_err(sd, "Invalid pll input clock: %d Hz\n", pllinclk);
> > + return -EINVAL;
> > + }
> > + state->pllinclk_hz = pllinclk;
> > +
> > + /* Now parse the fw-node */
> > + fwnode_graph_for_each_endpoint(dev_fwnode(dev), fw_node) {
> > + struct fwnode_endpoint fw_ep;
> > +
> > + ret = fwnode_graph_parse_endpoint(fw_node, &fw_ep);
> > + if (ret)
> > + return -EINVAL;
> > +
> > + /* Get downstream endpoint */
> > + if (fw_ep.port == 1)
> > + break;
>
> You could use fwnode_graph_get_endpoint_by_id() instead of the loop.
Of course.
> > + }
> > +
> > + if (!fw_node) {
> > + v4l2_err(sd, "Missing endpoint node\n");
> > + return -EINVAL;
> > + }
> > +
> > + ret = v4l2_fwnode_endpoint_alloc_parse(fw_node, &endpoint);
> > + if (ret) {
> > + v4l2_err(sd, "Failed to parse endpoint %d\n", ret);
> > + return ret;
> > + }
> > +
> > + state->csi_dir = TC358746_CSI_TX;
> > +
> > + if (endpoint.bus.mipi_csi2.num_data_lanes == 0 ||
> > + endpoint.nr_of_link_frequencies == 0) {
> > + v4l2_err(sd, "Missing CSI-2 properties in endpoint\n");
> > + ret = -EINVAL;
> > + goto free_ep;
> > + }
> > +
> > + if (endpoint.bus.mipi_csi2.num_data_lanes > 4) {
> > + v4l2_err(sd, "Invalid number of lanes\n");
> > + ret = -EINVAL;
> > + goto free_ep;
> > + }
> > +
> > + state->link_freq_settings =
> > + devm_kcalloc(dev, endpoint.nr_of_link_frequencies,
> > + sizeof(*state->link_freq_settings), GFP_KERNEL);
> > + if (!state->link_freq_settings) {
> > + ret = -ENOMEM;
> > + goto free_ep;
> > + }
> > +
> > + state->link_frequencies =
> > + devm_kcalloc(dev, endpoint.nr_of_link_frequencies,
> > + sizeof(*state->link_frequencies), GFP_KERNEL);
> > + if (!state->link_frequencies) {
> > + ret = -ENOMEM;
> > + goto free_ep;
> > + }
> > +
> > + ret = tc358746_set_lane_settings(state, &endpoint);
> > + if (ret)
> > + goto free_ep;
> > +
> > + state->reset_gpio = devm_gpiod_get_optional(dev, "reset",
> > + GPIOD_OUT_LOW);
> > + if (IS_ERR(state->reset_gpio)) {
> > + v4l2_err(sd, "failed to get reset gpio\n");
> > + ret = PTR_ERR(state->reset_gpio);
> > + goto free_ep;
> > + }
> > +
> > + ret = 0;
> > +
> > +free_ep:
> > + v4l2_fwnode_endpoint_free(&endpoint);
> > + return ret;
> > +}
> > +
> > +static int tc358746_parse_endpoint(struct device *dev,
> > + struct v4l2_fwnode_endpoint *vep,
> > + struct v4l2_async_subdev *asd)
> > +{
> > + struct v4l2_subdev *sd = dev_get_drvdata(dev);
> > + struct tc358746_state *state = to_state(sd);
> > +
> > + if (!fwnode_device_is_available(asd->match.fwnode)) {
> > + v4l2_err(sd, "remote is not available\n");
> > + return -ENOTCONN;
> > + }
> > +
> > + if (state->csi_dir == TC358746_CSI_TX &&
> > + vep->bus_type != V4L2_MBUS_PARALLEL) {
> > + v4l2_err(sd, "invalid bus type, must be PARALLEL\n");
> > + return -ENOTCONN;
> > + }
> > +
> > + return 0;
> > +};
> > +
> > +static int tc358746_async_register(struct v4l2_subdev *sd)
> > +{
> > + unsigned int port = 0;
> > +
> > + return v4l2_async_register_fwnode_subdev(
> > + sd, sizeof(struct v4l2_async_subdev), &port, 1,
> > + tc358746_parse_endpoint);
>
> Please move this to the caller instead.
Okay.
> > +
> > +}
> > +
> > +static const char * const tc358764_test_pattern_menu[] = {
> > + "Disabled",
> > + "80 Pixel Color Bars",
> > +};
> > +
> > +static int tc358746_probe(struct i2c_client *client)
> > +{
> > + struct tc358746_state *state;
> > + struct v4l2_subdev *sd;
> > + int err;
> > +
> > + if (!i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_BYTE_DATA))
> > + return -EIO;
> > +
> > + state = devm_kzalloc(&client->dev, sizeof(*state), GFP_KERNEL);
> > + if (!state)
> > + return -ENOMEM;
> > +
> > + state->i2c_client = client;
>
> You can use v4l2_get_subdevdata() to obtain the I�C client; no need to
> store it separately.
Yes, your're right. I change that.
> > + sd = &state->sd;
> > + v4l2_i2c_subdev_init(sd, client, &tc358746_ops);
> > + sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
> > +
> > + err = tc358746_probe_fw(state);
> > + if (err)
> > + return err;
> > +
> > + err = tc358746_apply_fw(state);
> > + if (err)
> > + return err;
> > +
> > + if (((i2c_rd16(sd, CHIPID) & CHIPID_CHIPID_MASK) >> 8) != 0x44) {
> > + v4l2_info(sd, "not a TC358746 on address 0x%x\n",
> > + client->addr << 1);
> > + err = -ENODEV;
> > + goto err_clk;
> > + }
> > +
> > + v4l2_ctrl_handler_init(&state->hdl, 2);
> > +
> > + v4l2_ctrl_new_std_menu_items(&state->hdl,
> > + &tc358764_ctrl_ops, V4L2_CID_TEST_PATTERN,
> > + ARRAY_SIZE(tc358764_test_pattern_menu) - 1, 0, 0,
> > + tc358764_test_pattern_menu);
> > +
> > + state->link_freq =
> > + v4l2_ctrl_new_int_menu(&state->hdl, &tc358764_ctrl_ops,
> > + V4L2_CID_LINK_FREQ,
> > + state->link_frequencies_num - 1,
> > + TC358746_DEF_LINK_FREQ,
> > + state->link_frequencies);
> > +
> > +
> > + sd->ctrl_handler = &state->hdl;
> > + if (state->hdl.error) {
> > + err = state->hdl.error;
> > + goto err_hdl;
> > + }
> > +
> > + state->pads[0].flags = MEDIA_PAD_FL_SINK;
> > + state->pads[1].flags = MEDIA_PAD_FL_SOURCE;
> > + sd->entity.function = MEDIA_ENT_F_VID_IF_BRIDGE;
> > + sd->entity.ops = &tc358746_entity_ops;
> > + err = media_entity_pads_init(&sd->entity, 2, state->pads);
> > + if (err < 0)
> > + goto err_hdl;
> > +
> > + mutex_init(&state->confctl_mutex);
> > +
> > + state->fmt = tc358746_def_fmt;
> > +
> > + /* Apply default settings */
> > + tc358746_sreset(sd);
> > + tc358746_set_buffers(sd);
> > + tc358746_set_csi(sd);
> > + tc358746_set_csi_color_space(sd);
> > + tc358746_sleep_mode(sd, 1);
> > + tc358746_set_pll(sd);
> > + tc358746_enable_stream(sd, 0);
> > +
> > + err = tc358746_async_register(sd);
> > + if (err < 0)
> > + goto err_mutex;
> > +
> > + v4l2_info(sd, "%s found @ 0x%x (%s)\n", client->name,
> > + client->addr << 1, client->adapter->name);
> > +
> > + return 0;
> > +
> > +err_mutex:
> > + mutex_destroy(&state->confctl_mutex);
> > +err_hdl:
> > + media_entity_cleanup(&sd->entity);
> > + v4l2_ctrl_handler_free(&state->hdl);
> > +err_clk:
> > + clk_disable_unprepare(state->refclk);
> > +
> > + return err;
> > +}
> > +
> > +static int tc358746_remove(struct i2c_client *client)
> > +{
> > + struct v4l2_subdev *sd = i2c_get_clientdata(client);
> > + struct tc358746_state *state = to_state(sd);
> > +
> > + v4l2_async_unregister_subdev(sd);
> > + v4l2_device_unregister_subdev(sd);
> > + mutex_destroy(&state->confctl_mutex);
> > + media_entity_cleanup(&sd->entity);
> > + v4l2_ctrl_handler_free(&state->hdl);
> > +
> > + return 0;
> > +}
> > +
> > +static const struct of_device_id __maybe_unused tc358746_of_match[] = {
> > + { .compatible = "toshiba,tc358746" },
> > + { },
> > +};
> > +MODULE_DEVICE_TABLE(of, tc358746_of_match);
> > +
> > +static struct i2c_driver tc358746_driver = {
> > + .driver = {
> > + .name = "tc358746",
> > + .of_match_table = of_match_ptr(tc358746_of_match),
>
> No need for of_patch_ptr() here.
Okay.
Thanks for the review =)
Regards,
Marco
>
> > + },
> > + .probe_new = tc358746_probe,
> > + .remove = tc358746_remove,
> > +};
> > +
> > +module_i2c_driver(tc358746_driver);
> > diff --git a/drivers/media/i2c/tc358746_regs.h b/drivers/media/i2c/tc358746_regs.h
> > new file mode 100644
> > index 000000000000..9232d00d0e92
> > --- /dev/null
> > +++ b/drivers/media/i2c/tc358746_regs.h
> > @@ -0,0 +1,208 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only */
> > +/*
> > + * tc358746 - Toshiba Parallel to CSI-2 bridge - register names and bit masks
> > + *
> > + * Convention:
> > + * <REGISTER>
> > + * <REGISTER>_<BITFIELD>_MASK
> > + * <REGISTER>_<BITFIELD>_<VALUE>
> > + * <REGISTER>_<BITFIELD>_SET(val = <REGISTER>_<BITFIELD>_<VALUE>)
> > + *
> > + * References:
> > + * REF_01:
> > + * - TC358746AXBG/TC358748XBG/TC358748IXBG Functional Specification Rev 1.2
> > + */
> > +
> > +#ifndef __TC358746_REGS_H
> > +#define __TC358746_REGS_H
> > +
> > +#define CHIPID 0x0000
> > +#define CHIPID_CHIPID_MASK GENMASK(15, 8)
> > +#define CHIPID_REVID_MASK GENMASK(7, 0)
> > +
> > +#define SYSCTL 0x0002
> > +#define SYSCTL_SLEEP_MASK BIT(1)
> > +#define SYSCTL_SRESET_MASK BIT(0)
> > +
> > +#define CONFCTL 0x0004
> > +#define CONFCTL_TRIEN_MASK BIT(15)
> > +#define CONFCTL_INTE2N_MASK BIT(13)
> > +#define CONFCTL_BT656EN_MASK BIT(12)
> > +#define CONFCTL_PDATAF_MASK GENMASK(9, 8)
> > +#define CONFCTL_PDATAF_SET(val) (((val << 8) & CONFCTL_PDATAF_MASK))
> > +#define CONFCTL_PDATAF_MODE0 0
> > +#define CONFCTL_PDATAF_MODE1 1
> > +#define CONFCTL_PDATAF_MODE2 2
> > +#define CONFCTL_PPEN_MASK BIT(6)
> > +#define CONFCTL_VVALIDP_MASK BIT(5)
> > +#define CONFCTL_HVALIDP_MASK BIT(4)
> > +#define CONFCTL_PCLKP_MASK BIT(3)
> > +#define CONFCTL_AUTO_MASK BIT(2)
> > +#define CONFCTL_DATALANE_MASK GENMASK(1, 0)
> > +#define CONFCTL_DATALANE_1 0
> > +#define CONFCTL_DATALANE_2 1
> > +#define CONFCTL_DATALANE_3 2
> > +#define CONFCTL_DATALANE_4 3
> > +
> > +#define FIFOCTL 0x0006
> > +#define DATAFMT 0x0008
> > +#define DATAFMT_PDFMT_RAW8 0
> > +#define DATAFMT_PDFMT_RAW10 1
> > +#define DATAFMT_PDFMT_RAW12 2
> > +#define DATAFMT_PDFMT_RGB888 3
> > +#define DATAFMT_PDFMT_RGB666 4
> > +#define DATAFMT_PDFMT_RGB565 5
> > +#define DATAFMT_PDFMT_YCBCRFMT_422_8_BIT 6
> > +#define DATAFMT_PDFMT_RAW14 8
> > +#define DATAFMT_PDFMT_YCBCRFMT_422_10_BIT 9
> > +#define DATAFMT_PDFMT_YCBCRFMT_444 10
> > +#define DATAFMT_PDFMT_MASK GENMASK(7, 4)
> > +#define DATAFMT_PDFMT_SET(val) (((val) << 4) & DATAFMT_PDFMT_MASK)
> > +#define DATAFMT_UDT_EN_MASK BIT(0)
> > +
> > +#define MCLKCTL 0x000c
> > +#define MCLKCTL_MCLK_HIGH_MASK GENMASK(15, 8)
> > +#define MCLKCTL_MCLK_HIGH_SET(val) ((((val) - 1) << 8) & MCLKCTL_MCLK_HIGH_MASK)
> > +#define MCLKCTL_MCLK_LOW_MASK GENMASK(7, 0)
> > +#define MCLKCTL_MCLK_LOW_SET(val) (((val) - 1) & MCLKCTL_MCLK_LOW_MASK)
> > +
> > +#define PLLCTL0 0x0016
> > +#define PLLCTL0_PLL_PRD_MASK GENMASK(15, 12)
> > +#define PLLCTL0_PLL_PRD_SET(prd) ((((prd) - 1) << 12) & PLLCTL0_PLL_PRD_MASK)
> > +#define PLLCTL0_PLL_FBD_MASK GENMASK(8, 0)
> > +#define PLLCTL0_PLL_FBD_SET(fbd) (((fbd) - 1) & PLLCTL0_PLL_FBD_MASK)
> > +
> > +#define PLLCTL1 0x0018
> > +#define PLLCTL1_PLL_FRS_MASK GENMASK(11, 10)
> > +#define PLLCTL1_PLL_FRS_SET(frs) (((frs) << 10) & PLLCTL1_PLL_FRS_MASK)
> > +#define PLLCTL1_PLL_LBWS_MASK GENMASK(9, 8)
> > +#define PLLCTL1_LFBREN_MASK BIT(6)
> > +#define PLLCTL1_BYPCKEN_MASK BIT(5)
> > +#define PLLCTL1_CKEN_MASK BIT(4)
> > +#define PLLCTL1_RESETB_MASK BIT(1)
> > +#define PLLCTL1_PLL_EN_MASK BIT(0)
> > +
> > +#define CLKCTL 0x0020
> > +#define CLKCTL_MCLKDIV_MASK GENMASK(3, 2)
> > +#define CLKCTL_MCLKDIV_SET(val) ((val << 2) & CLKCTL_MCLKDIV_MASK)
> > +#define CLKCTL_MCLKDIV_8 0
> > +#define CLKCTL_MCLKDIV_4 1
> > +#define CLKCTL_MCLKDIV_2 2
> > +
> > +#define WORDCNT 0x0022
> > +#define PP_MISC 0x0032
> > +#define PP_MISC_FRMSTOP_MASK BIT(15)
> > +#define PP_MISC_RSTPTR_MASK BIT(14)
> > +
> > +#define CSI2TX_DATA_TYPE 0x0050
> > +#define MIPI_PHY_STATUS 0x0062
> > +#define CSI2_ERROR_STATUS 0x0064
> > +#define CSI2_ERR_EN 0x0066
> > +#define CSI2_IDID_ERROR 0x006c
> > +#define DBG_ACT_LINE_CNT 0x00e0
> > +#define DBG_LINE_WIDTH 0x00e2
> > +#define DBG_VERT_BLANK_LINE_CNT 0x00e4
> > +#define DBG_VIDEO_DATA 0x00e8
> > +#define FIFOSTATUS 0x00F8
> > +
> > +#define CLW_CNTRL 0x0140
> > +#define CLW_CNTRL_CLW_LANEDISABLE_MASK BIT(0)
> > +
> > +#define D0W_CNTRL 0x0144
> > +#define D0W_CNTRL_D0W_LANEDISABLE_MASK BIT(0)
> > +
> > +#define D1W_CNTRL 0x0148
> > +#define D1W_CNTRL_D1W_LANEDISABLE_MASK BIT(0)
> > +
> > +#define D2W_CNTRL 0x014C
> > +#define D2W_CNTRL_D2W_LANEDISABLE_MASK BIT(0)
> > +
> > +#define D3W_CNTRL 0x0150
> > +#define D2W_CNTRL_D3W_LANEDISABLE_MASK BIT(0)
> > +
> > +#define STARTCNTRL 0x0204
> > +#define STARTCNTRL_START_MASK BIT(0)
> > +
> > +#define LINEINITCNT 0x0210
> > +#define LPTXTIMECNT 0x0214
> > +#define TCLK_HEADERCNT 0x0218
> > +#define TCLK_HEADERCNT_TCLK_ZEROCNT_MASK GENMASK(15, 8)
> > +#define TCLK_HEADERCNT_TCLK_PREPARECNT_MASK GENMASK(6, 0)
> > +#define TCLK_HEADERCNT_TCLK_ZEROCNT_SET(val) ((val << 8) & TCLK_HEADERCNT_TCLK_ZEROCNT_MASK)
> > +#define TCLK_HEADERCNT_TCLK_PREPARECNT_SET(val) (val & TCLK_HEADERCNT_TCLK_PREPARECNT_MASK)
> > +
> > +#define TCLK_TRAILCNT 0x021C
> > +#define THS_HEADERCNT 0x0220
> > +#define THS_HEADERCNT_THS_ZEROCNT_MASK GENMASK(14, 8)
> > +#define THS_HEADERCNT_THS_PREPARECNT_MASK GENMASK(6, 0)
> > +#define THS_HEADERCNT_THS_ZEROCNT_SET(val) ((val << 8) & THS_HEADERCNT_THS_ZEROCNT_MASK)
> > +#define THS_HEADERCNT_THS_PREPARECNT_SET(val) (val & THS_HEADERCNT_THS_PREPARECNT_MASK)
> > +
> > +#define TWAKEUP 0x0224
> > +#define TCLK_POSTCNT 0x0228
> > +#define THS_TRAILCNT 0x022C
> > +#define HSTXVREGCNT 0x0230
> > +#define HSTXVREGEN 0x0234
> > +#define HSTXVREGEN_D3M_HSTXVREGEN_MASK BIT(4)
> > +#define HSTXVREGEN_D2M_HSTXVREGEN_MASK BIT(3)
> > +#define HSTXVREGEN_D1M_HSTXVREGEN_MASK BIT(2)
> > +#define HSTXVREGEN_D0M_HSTXVREGEN_MASK BIT(1)
> > +#define HSTXVREGEN_CLM_HSTXVREGEN_MASK BIT(0)
> > +
> > +#define TXOPTIONCNTRL 0x0238
> > +#define TXOPTIONCNTRL_CONTCLKMODE_MASK BIT(0)
> > +
> > +#define CSI_CONTROL 0x040C
> > +#define CSI_CONTROL_CSI_MODE_MASK BIT(15)
> > +#define CSI_CONTROL_HTXTOEN_MASK BIT(10)
> > +#define CSI_CONTROL_TXHSMD_MASK BIT(7)
> > +#define CSI_CONTROL_NOL_MASK GENMASK(2, 1)
> > +#define CSI_CONTROL_NOL_1_MASK 0
> > +#define CSI_CONTROL_NOL_2_MASK BIT(1)
> > +#define CSI_CONTROL_NOL_3_MASK BIT(2)
> > +#define CSI_CONTROL_NOL_4_MASK (BIT(1) | BIT(2))
> > +#define CSI_CONTROL_EOTDIS_MASK BIT(0)
> > +
> > +#define CSI_STATUS 0x0410
> > +#define CSI_STATUS_S_WSYNC_MASK BIT(10)
> > +#define CSI_STATUS_S_TXACT_MASK BIT(9)
> > +#define CSI_STATUS_S_HLT_MASK BIT(0)
> > +
> > +#define CSI_INT 0x0414
> > +#define CSI_INT_INTHLT_MASK BIT(3)
> > +#define CSI_INT_INTER_MASK BIT(2)
> > +
> > +#define CSI_INT_ENA 0x0418
> > +#define CSI_INT_ENA_IENHLT_MASK BIT(3)
> > +#define CSI_INT_ENA_IENER_MASK BIT(2)
> > +
> > +#define CSI_ERR 0x044C
> > +#define CSI_ERR_INER_MASK BIT(9)
> > +#define CSI_ERR_WCER_MASK BIT(8)
> > +#define CSI_ERR_QUNK_MASK BIT(4)
> > +#define CSI_ERR_TXBRK_MASK BIT(1)
> > +
> > +#define CSI_ERR_INTENA 0x0450
> > +#define CSI_ERR_HALT 0x0454
> > +#define CSI_CONFW 0x0500
> > +#define CSI_CONFW_MODE_MASK GENMASK(31, 29)
> > +#define CSI_CONFW_MODE_SET_MASK (BIT(31) | BIT(29))
> > +#define CSI_CONFW_MODE_CLEAR_MASK (BIT(31) | BIT(30))
> > +#define CSI_CONFW_ADDRESS_MASK GENMASK(28, 24)
> > +#define CSI_CONFW_ADDRESS_CSI_CONTROL_MASK (BIT(24) | BIT(25))
> > +#define CSI_CONFW_ADDRESS_CSI_INT_ENA_MASK (BIT(25) | BIT(26))
> > +#define CSI_CONFW_ADDRESS_CSI_ERR_INTENA_MASK (BIT(28) | BIT(26))
> > +#define CSI_CONFW_ADDRESS_CSI_ERR_HALT_MASK (BIT(28) | BIT(26) | BIT(24))
> > +#define CSI_CONFW_DATA_MASK GENMASK(15, 0)
> > +
> > +#define CSIRESET 0x0504
> > +#define CSIRESET_RESET_CNF_MASK BIT(1)
> > +#define CSIRESET_RESET_MODULE_MASK BIT(0)
> > +
> > +#define CSI_INT_CLR 0x050C
> > +#define CSI_INT_CLR_ICRER_MASK BIT(2)
> > +
> > +#define CSI_START 0x0518
> > +#define CSI_START_STRT_MASK BIT(0)
> > +
> > +#endif
>
> --
> Kind regards,
>
> Sakari Ailus
> sakari.ailus@linux.intel.com
>
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply
* Re: [PATCH 08/11] power/supply: Drop obsolete JZ4740 driver
From: Sebastian Reichel @ 2019-07-29 11:03 UTC (permalink / raw)
To: Paul Cercueil
Cc: Mark Rutland, linux-fbdev, James Hogan, alsa-devel, dri-devel,
Liam Girdwood, od, linux-mtd, Miquel Raynal, Lee Jones,
Artur Rojek, Richard Weinberger, linux-pm, Paul Burton,
linux-mips, Guenter Roeck, devicetree, Jean Delvare,
Bartlomiej Zolnierkiewicz, Mark Brown, linux-hwmon, linux-kernel,
Ralf Baechle, Vinod Koul, Rob Herring, dmaengine
In-Reply-To: <20190725220215.460-9-paul@crapouillou.net>
[-- Attachment #1.1: Type: text/plain, Size: 14573 bytes --]
Hi,
On Thu, Jul 25, 2019 at 06:02:12PM -0400, Paul Cercueil wrote:
> It has been replaced with the more mature ingenic-battery driver.
>
> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> Tested-by: Artur Rojek <contact@artur-rojek.eu>
> ---
Acked-by: Sebastian Reichel <sebastian.reichel@collabora.com>
-- Sebastian
> drivers/power/supply/Kconfig | 11 -
> drivers/power/supply/Makefile | 1 -
> drivers/power/supply/jz4740-battery.c | 421 --------------------------
> 3 files changed, 433 deletions(-)
> delete mode 100644 drivers/power/supply/jz4740-battery.c
>
> diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
> index 5d91b5160b41..6ba602ed7979 100644
> --- a/drivers/power/supply/Kconfig
> +++ b/drivers/power/supply/Kconfig
> @@ -417,17 +417,6 @@ config CHARGER_PCF50633
> help
> Say Y to include support for NXP PCF50633 Main Battery Charger.
>
> -config BATTERY_JZ4740
> - tristate "Ingenic JZ4740 battery"
> - depends on MACH_JZ4740
> - depends on MFD_JZ4740_ADC
> - help
> - Say Y to enable support for the battery on Ingenic JZ4740 based
> - boards.
> -
> - This driver can be build as a module. If so, the module will be
> - called jz4740-battery.
> -
> config BATTERY_RX51
> tristate "Nokia RX-51 (N900) battery driver"
> depends on TWL4030_MADC
> diff --git a/drivers/power/supply/Makefile b/drivers/power/supply/Makefile
> index 96c2b74b36bf..6c7da920ea83 100644
> --- a/drivers/power/supply/Makefile
> +++ b/drivers/power/supply/Makefile
> @@ -58,7 +58,6 @@ obj-$(CONFIG_BATTERY_S3C_ADC) += s3c_adc_battery.o
> obj-$(CONFIG_BATTERY_TWL4030_MADC) += twl4030_madc_battery.o
> obj-$(CONFIG_CHARGER_88PM860X) += 88pm860x_charger.o
> obj-$(CONFIG_CHARGER_PCF50633) += pcf50633-charger.o
> -obj-$(CONFIG_BATTERY_JZ4740) += jz4740-battery.o
> obj-$(CONFIG_BATTERY_RX51) += rx51_battery.o
> obj-$(CONFIG_AB8500_BM) += ab8500_bmdata.o ab8500_charger.o ab8500_fg.o ab8500_btemp.o abx500_chargalg.o pm2301_charger.o
> obj-$(CONFIG_CHARGER_CPCAP) += cpcap-charger.o
> diff --git a/drivers/power/supply/jz4740-battery.c b/drivers/power/supply/jz4740-battery.c
> deleted file mode 100644
> index 6366bd61ea9f..000000000000
> --- a/drivers/power/supply/jz4740-battery.c
> +++ /dev/null
> @@ -1,421 +0,0 @@
> -// SPDX-License-Identifier: GPL-2.0-only
> -/*
> - * Battery measurement code for Ingenic JZ SOC.
> - *
> - * Copyright (C) 2009 Jiejing Zhang <kzjeef@gmail.com>
> - * Copyright (C) 2010, Lars-Peter Clausen <lars@metafoo.de>
> - *
> - * based on tosa_battery.c
> - *
> - * Copyright (C) 2008 Marek Vasut <marek.vasut@gmail.com>
> - */
> -
> -#include <linux/interrupt.h>
> -#include <linux/kernel.h>
> -#include <linux/module.h>
> -#include <linux/platform_device.h>
> -#include <linux/slab.h>
> -#include <linux/io.h>
> -
> -#include <linux/delay.h>
> -#include <linux/err.h>
> -#include <linux/gpio.h>
> -#include <linux/mfd/core.h>
> -#include <linux/power_supply.h>
> -
> -#include <linux/power/jz4740-battery.h>
> -#include <linux/jz4740-adc.h>
> -
> -struct jz_battery {
> - struct jz_battery_platform_data *pdata;
> - struct platform_device *pdev;
> -
> - void __iomem *base;
> -
> - int irq;
> - int charge_irq;
> -
> - const struct mfd_cell *cell;
> -
> - int status;
> - long voltage;
> -
> - struct completion read_completion;
> -
> - struct power_supply *battery;
> - struct power_supply_desc battery_desc;
> - struct delayed_work work;
> -
> - struct mutex lock;
> -};
> -
> -static inline struct jz_battery *psy_to_jz_battery(struct power_supply *psy)
> -{
> - return power_supply_get_drvdata(psy);
> -}
> -
> -static irqreturn_t jz_battery_irq_handler(int irq, void *devid)
> -{
> - struct jz_battery *battery = devid;
> -
> - complete(&battery->read_completion);
> - return IRQ_HANDLED;
> -}
> -
> -static long jz_battery_read_voltage(struct jz_battery *battery)
> -{
> - long t;
> - unsigned long val;
> - long voltage;
> -
> - mutex_lock(&battery->lock);
> -
> - reinit_completion(&battery->read_completion);
> -
> - enable_irq(battery->irq);
> - battery->cell->enable(battery->pdev);
> -
> - t = wait_for_completion_interruptible_timeout(&battery->read_completion,
> - HZ);
> -
> - if (t > 0) {
> - val = readw(battery->base) & 0xfff;
> -
> - if (battery->pdata->info.voltage_max_design <= 2500000)
> - val = (val * 78125UL) >> 7UL;
> - else
> - val = ((val * 924375UL) >> 9UL) + 33000;
> - voltage = (long)val;
> - } else {
> - voltage = t ? t : -ETIMEDOUT;
> - }
> -
> - battery->cell->disable(battery->pdev);
> - disable_irq(battery->irq);
> -
> - mutex_unlock(&battery->lock);
> -
> - return voltage;
> -}
> -
> -static int jz_battery_get_capacity(struct power_supply *psy)
> -{
> - struct jz_battery *jz_battery = psy_to_jz_battery(psy);
> - struct power_supply_info *info = &jz_battery->pdata->info;
> - long voltage;
> - int ret;
> - int voltage_span;
> -
> - voltage = jz_battery_read_voltage(jz_battery);
> -
> - if (voltage < 0)
> - return voltage;
> -
> - voltage_span = info->voltage_max_design - info->voltage_min_design;
> - ret = ((voltage - info->voltage_min_design) * 100) / voltage_span;
> -
> - if (ret > 100)
> - ret = 100;
> - else if (ret < 0)
> - ret = 0;
> -
> - return ret;
> -}
> -
> -static int jz_battery_get_property(struct power_supply *psy,
> - enum power_supply_property psp, union power_supply_propval *val)
> -{
> - struct jz_battery *jz_battery = psy_to_jz_battery(psy);
> - struct power_supply_info *info = &jz_battery->pdata->info;
> - long voltage;
> -
> - switch (psp) {
> - case POWER_SUPPLY_PROP_STATUS:
> - val->intval = jz_battery->status;
> - break;
> - case POWER_SUPPLY_PROP_TECHNOLOGY:
> - val->intval = jz_battery->pdata->info.technology;
> - break;
> - case POWER_SUPPLY_PROP_HEALTH:
> - voltage = jz_battery_read_voltage(jz_battery);
> - if (voltage < info->voltage_min_design)
> - val->intval = POWER_SUPPLY_HEALTH_DEAD;
> - else
> - val->intval = POWER_SUPPLY_HEALTH_GOOD;
> - break;
> - case POWER_SUPPLY_PROP_CAPACITY:
> - val->intval = jz_battery_get_capacity(psy);
> - break;
> - case POWER_SUPPLY_PROP_VOLTAGE_NOW:
> - val->intval = jz_battery_read_voltage(jz_battery);
> - if (val->intval < 0)
> - return val->intval;
> - break;
> - case POWER_SUPPLY_PROP_VOLTAGE_MAX_DESIGN:
> - val->intval = info->voltage_max_design;
> - break;
> - case POWER_SUPPLY_PROP_VOLTAGE_MIN_DESIGN:
> - val->intval = info->voltage_min_design;
> - break;
> - case POWER_SUPPLY_PROP_PRESENT:
> - val->intval = 1;
> - break;
> - default:
> - return -EINVAL;
> - }
> - return 0;
> -}
> -
> -static void jz_battery_external_power_changed(struct power_supply *psy)
> -{
> - struct jz_battery *jz_battery = psy_to_jz_battery(psy);
> -
> - mod_delayed_work(system_wq, &jz_battery->work, 0);
> -}
> -
> -static irqreturn_t jz_battery_charge_irq(int irq, void *data)
> -{
> - struct jz_battery *jz_battery = data;
> -
> - mod_delayed_work(system_wq, &jz_battery->work, 0);
> -
> - return IRQ_HANDLED;
> -}
> -
> -static void jz_battery_update(struct jz_battery *jz_battery)
> -{
> - int status;
> - long voltage;
> - bool has_changed = false;
> - int is_charging;
> -
> - if (gpio_is_valid(jz_battery->pdata->gpio_charge)) {
> - is_charging = gpio_get_value(jz_battery->pdata->gpio_charge);
> - is_charging ^= jz_battery->pdata->gpio_charge_active_low;
> - if (is_charging)
> - status = POWER_SUPPLY_STATUS_CHARGING;
> - else
> - status = POWER_SUPPLY_STATUS_NOT_CHARGING;
> -
> - if (status != jz_battery->status) {
> - jz_battery->status = status;
> - has_changed = true;
> - }
> - }
> -
> - voltage = jz_battery_read_voltage(jz_battery);
> - if (voltage >= 0 && abs(voltage - jz_battery->voltage) > 50000) {
> - jz_battery->voltage = voltage;
> - has_changed = true;
> - }
> -
> - if (has_changed)
> - power_supply_changed(jz_battery->battery);
> -}
> -
> -static enum power_supply_property jz_battery_properties[] = {
> - POWER_SUPPLY_PROP_STATUS,
> - POWER_SUPPLY_PROP_TECHNOLOGY,
> - POWER_SUPPLY_PROP_HEALTH,
> - POWER_SUPPLY_PROP_CAPACITY,
> - POWER_SUPPLY_PROP_VOLTAGE_NOW,
> - POWER_SUPPLY_PROP_VOLTAGE_MAX_DESIGN,
> - POWER_SUPPLY_PROP_VOLTAGE_MIN_DESIGN,
> - POWER_SUPPLY_PROP_PRESENT,
> -};
> -
> -static void jz_battery_work(struct work_struct *work)
> -{
> - /* Too small interval will increase system workload */
> - const int interval = HZ * 30;
> - struct jz_battery *jz_battery = container_of(work, struct jz_battery,
> - work.work);
> -
> - jz_battery_update(jz_battery);
> - schedule_delayed_work(&jz_battery->work, interval);
> -}
> -
> -static int jz_battery_probe(struct platform_device *pdev)
> -{
> - int ret = 0;
> - struct jz_battery_platform_data *pdata = pdev->dev.parent->platform_data;
> - struct power_supply_config psy_cfg = {};
> - struct jz_battery *jz_battery;
> - struct power_supply_desc *battery_desc;
> - struct resource *mem;
> -
> - if (!pdata) {
> - dev_err(&pdev->dev, "No platform_data supplied\n");
> - return -ENXIO;
> - }
> -
> - jz_battery = devm_kzalloc(&pdev->dev, sizeof(*jz_battery), GFP_KERNEL);
> - if (!jz_battery) {
> - dev_err(&pdev->dev, "Failed to allocate driver structure\n");
> - return -ENOMEM;
> - }
> -
> - jz_battery->cell = mfd_get_cell(pdev);
> -
> - jz_battery->irq = platform_get_irq(pdev, 0);
> - if (jz_battery->irq < 0) {
> - dev_err(&pdev->dev, "Failed to get platform irq: %d\n", ret);
> - return jz_battery->irq;
> - }
> -
> - mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> -
> - jz_battery->base = devm_ioremap_resource(&pdev->dev, mem);
> - if (IS_ERR(jz_battery->base))
> - return PTR_ERR(jz_battery->base);
> -
> - battery_desc = &jz_battery->battery_desc;
> - battery_desc->name = pdata->info.name;
> - battery_desc->type = POWER_SUPPLY_TYPE_BATTERY;
> - battery_desc->properties = jz_battery_properties;
> - battery_desc->num_properties = ARRAY_SIZE(jz_battery_properties);
> - battery_desc->get_property = jz_battery_get_property;
> - battery_desc->external_power_changed =
> - jz_battery_external_power_changed;
> - battery_desc->use_for_apm = 1;
> -
> - psy_cfg.drv_data = jz_battery;
> -
> - jz_battery->pdata = pdata;
> - jz_battery->pdev = pdev;
> -
> - init_completion(&jz_battery->read_completion);
> - mutex_init(&jz_battery->lock);
> -
> - INIT_DELAYED_WORK(&jz_battery->work, jz_battery_work);
> -
> - ret = request_irq(jz_battery->irq, jz_battery_irq_handler, 0, pdev->name,
> - jz_battery);
> - if (ret) {
> - dev_err(&pdev->dev, "Failed to request irq %d\n", ret);
> - return ret;
> - }
> - disable_irq(jz_battery->irq);
> -
> - if (gpio_is_valid(pdata->gpio_charge)) {
> - ret = gpio_request(pdata->gpio_charge, dev_name(&pdev->dev));
> - if (ret) {
> - dev_err(&pdev->dev, "charger state gpio request failed.\n");
> - goto err_free_irq;
> - }
> - ret = gpio_direction_input(pdata->gpio_charge);
> - if (ret) {
> - dev_err(&pdev->dev, "charger state gpio set direction failed.\n");
> - goto err_free_gpio;
> - }
> -
> - jz_battery->charge_irq = gpio_to_irq(pdata->gpio_charge);
> -
> - if (jz_battery->charge_irq >= 0) {
> - ret = request_irq(jz_battery->charge_irq,
> - jz_battery_charge_irq,
> - IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING,
> - dev_name(&pdev->dev), jz_battery);
> - if (ret) {
> - dev_err(&pdev->dev, "Failed to request charge irq: %d\n", ret);
> - goto err_free_gpio;
> - }
> - }
> - } else {
> - jz_battery->charge_irq = -1;
> - }
> -
> - if (jz_battery->pdata->info.voltage_max_design <= 2500000)
> - jz4740_adc_set_config(pdev->dev.parent, JZ_ADC_CONFIG_BAT_MB,
> - JZ_ADC_CONFIG_BAT_MB);
> - else
> - jz4740_adc_set_config(pdev->dev.parent, JZ_ADC_CONFIG_BAT_MB, 0);
> -
> - jz_battery->battery = power_supply_register(&pdev->dev, battery_desc,
> - &psy_cfg);
> - if (IS_ERR(jz_battery->battery)) {
> - dev_err(&pdev->dev, "power supply battery register failed.\n");
> - ret = PTR_ERR(jz_battery->battery);
> - goto err_free_charge_irq;
> - }
> -
> - platform_set_drvdata(pdev, jz_battery);
> - schedule_delayed_work(&jz_battery->work, 0);
> -
> - return 0;
> -
> -err_free_charge_irq:
> - if (jz_battery->charge_irq >= 0)
> - free_irq(jz_battery->charge_irq, jz_battery);
> -err_free_gpio:
> - if (gpio_is_valid(pdata->gpio_charge))
> - gpio_free(jz_battery->pdata->gpio_charge);
> -err_free_irq:
> - free_irq(jz_battery->irq, jz_battery);
> - return ret;
> -}
> -
> -static int jz_battery_remove(struct platform_device *pdev)
> -{
> - struct jz_battery *jz_battery = platform_get_drvdata(pdev);
> -
> - cancel_delayed_work_sync(&jz_battery->work);
> -
> - if (gpio_is_valid(jz_battery->pdata->gpio_charge)) {
> - if (jz_battery->charge_irq >= 0)
> - free_irq(jz_battery->charge_irq, jz_battery);
> - gpio_free(jz_battery->pdata->gpio_charge);
> - }
> -
> - power_supply_unregister(jz_battery->battery);
> -
> - free_irq(jz_battery->irq, jz_battery);
> -
> - return 0;
> -}
> -
> -#ifdef CONFIG_PM
> -static int jz_battery_suspend(struct device *dev)
> -{
> - struct jz_battery *jz_battery = dev_get_drvdata(dev);
> -
> - cancel_delayed_work_sync(&jz_battery->work);
> - jz_battery->status = POWER_SUPPLY_STATUS_UNKNOWN;
> -
> - return 0;
> -}
> -
> -static int jz_battery_resume(struct device *dev)
> -{
> - struct jz_battery *jz_battery = dev_get_drvdata(dev);
> -
> - schedule_delayed_work(&jz_battery->work, 0);
> -
> - return 0;
> -}
> -
> -static const struct dev_pm_ops jz_battery_pm_ops = {
> - .suspend = jz_battery_suspend,
> - .resume = jz_battery_resume,
> -};
> -
> -#define JZ_BATTERY_PM_OPS (&jz_battery_pm_ops)
> -#else
> -#define JZ_BATTERY_PM_OPS NULL
> -#endif
> -
> -static struct platform_driver jz_battery_driver = {
> - .probe = jz_battery_probe,
> - .remove = jz_battery_remove,
> - .driver = {
> - .name = "jz4740-battery",
> - .pm = JZ_BATTERY_PM_OPS,
> - },
> -};
> -
> -module_platform_driver(jz_battery_driver);
> -
> -MODULE_ALIAS("platform:jz4740-battery");
> -MODULE_LICENSE("GPL");
> -MODULE_AUTHOR("Lars-Peter Clausen <lars@metafoo.de>");
> -MODULE_DESCRIPTION("JZ4740 SoC battery driver");
> --
> 2.21.0.593.g511ec345e18
>
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
[-- Attachment #2: Type: text/plain, Size: 159 bytes --]
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: [PATCH V4 2/2] gpio: inverter: document the inverter bindings
From: Harish Jenny K N @ 2019-07-29 11:07 UTC (permalink / raw)
To: Rob Herring, Linus Walleij
Cc: Bartosz Golaszewski, Mark Rutland, devicetree,
open list:GPIO SUBSYSTEM <linux-gpio@vger.kernel.org>, Balasubramani Vivekanandan
In-Reply-To: <ec343df3-b374-fecf-6973-3b37614975a7@mentor.com>
Hi Linus,
On 17/07/19 7:21 PM, Harish Jenny K N wrote:
> Hi Linus,
>
> On 10/07/19 1:58 PM, Harish Jenny K N wrote:
>> Hi,
>>
>> On 09/07/19 9:38 PM, Rob Herring wrote:
>>> On Mon, Jul 8, 2019 at 11:25 PM Harish Jenny K N
>>> <harish_kandiga@mentor.com> wrote:
>>>> Hi Rob,
>>>>
>>>>
>>>> On 09/07/19 4:06 AM, Rob Herring wrote:
>>>>> On Fri, Jun 28, 2019 at 3:31 AM Harish Jenny K N
>>>>> <harish_kandiga@mentor.com> wrote:
>>>>>> Document the device tree binding for the inverter gpio
>>>>>> controller to configure the polarity of the gpio pins
>>>>>> used by the consumers.
>>>>>>
>>>>>> Signed-off-by: Harish Jenny K N <harish_kandiga@mentor.com>
>>>>>> ---
>>>>>> .../devicetree/bindings/gpio/gpio-inverter.txt | 29 ++++++++++++++++++++++
>>>>>> 1 file changed, 29 insertions(+)
>>>>>> create mode 100644 Documentation/devicetree/bindings/gpio/gpio-inverter.txt
>>>>>>
>>>>>> diff --git a/Documentation/devicetree/bindings/gpio/gpio-inverter.txt b/Documentation/devicetree/bindings/gpio/gpio-inverter.txt
>>>>>> new file mode 100644
>>>>>> index 0000000..8bb6b2e
>>>>>> --- /dev/null
>>>>>> +++ b/Documentation/devicetree/bindings/gpio/gpio-inverter.txt
>>>>>> @@ -0,0 +1,29 @@
>>>>>> +GPIO-INVERTER
>>>>>> +======
>>>>>> +This binding defines the gpio-inverter. The gpio-inverter is a driver that
>>>>>> +allows to properly describe the gpio polarities on the hardware.
>>>>> I don't understand. Please explain this in terms of the hardware, not a driver.
>>>> gpio inverters can be used on different hardware to alter the polarity of gpio chips.
>>>> The polarity of pins can change from hardware to hardware with the use of inverters.
>>> Yes, I know what an inverter is.
>>>
>>>> This device tree binding models gpio inverters in the device tree to properly describe the hardware.
>>> We already define the active state of GPIOs in the consumers. If
>>> there's an inverter in the middle, the consumer active state is simply
>>> inverted. I don't agree that that is a hack as Linus said without some
>>> reasoning why an inverter needs to be modeled in DT. Anything about
>>> what 'userspace' needs is not a reason. That's a Linux thing that has
>>> little to do with hardware description.
>> Yes we are talking about the hardware level inversions here. The usecase is for those without the gpio consumer driver. The usecase started with the concept of allowing an abstraction of the underlying hardware for the userland controlling program such that this program does not care whether the GPIO lines are inverted or not physically. In other words, a single userland controlling program can work unmodified across a variety of hardware platforms with the device tree mapping the logical to physical relationship of the GPIO hardware.
>> I totally understand anything about what 'userspace' needs is not a reason, but this is not restricted to userspace alone as kernel drivers may need this just as much. Also we are just modelling/describing the hardware state in the device tree.
>>
>> Just to mention that Linus Walleij had proposed this inverter model to describe the hardware and the gpio inverter driver is developed based on comments/review from him.
>>
>> Also my sincere request to Linus Walleij to please let his opinion know on this.
>>
>> Thanks,
>>
>> Best Regards,
>> Harish Jenny K N
>
> Can you please give your opinion on this.
>
>
> Thanks.
>
>
> Best Regards,
>
> Harish Jenny K N
>
sorry for the repeated mail. can you please give your opinion on this ?
Thanks.
Harish
^ permalink raw reply
* Re: [PATCH 00/11] JZ4740 SoC cleanup
From: Richard Weinberger @ 2019-07-29 11:23 UTC (permalink / raw)
To: Paul Cercueil
Cc: Ralf Baechle, Paul Burton, James Hogan, Rob Herring, Mark Rutland,
Vinod Koul, Jean Delvare, Guenter Roeck, Lee Jones, Miquel Raynal,
Richard Weinberger, Sebastian Reichel, Bartlomiej Zolnierkiewicz,
Liam Girdwood, Mark Brown, linux-hwmon, devicetree, linux-fbdev,
alsa-devel, linux-pm, linux-mips
In-Reply-To: <20190725220215.460-1-paul@crapouillou.net>
On Fri, Jul 26, 2019 at 12:02 AM Paul Cercueil <paul@crapouillou.net> wrote:
>
> Hi,
>
> This patchset converts the Qi LB60 MIPS board to devicetree and makes it
> use all the shiny new drivers that have been developed or updated
> recently.
>
> All the crappy old drivers and custom code can be dropped since they
> have been replaced by better alternatives.
>
> Some of these alternatives are not yet in 5.3-rc1 but have already been
> accepted by their respective maintainer for inclusion in 5.4-rc1.
>
> To upstream this patchset, I think that as soon as MIPS maintainers
> agree to take patches 01-03/11 and 11/11, the other patches can go
> through their respective maintainer's tree.
Was this series tested with the Ben Nanonote device?
I have one of these and from time to time I upgrade the kernel on it.
--
Thanks,
//richard
^ permalink raw reply
* Re: [PATCH 1/4] dt-bindings: i2c: sh_mobile: Rename bindings documentation file
From: Geert Uytterhoeven @ 2019-07-29 11:39 UTC (permalink / raw)
To: Simon Horman
Cc: Wolfram Sang, Chris Brandt, Rob Herring, Mark Rutland,
Magnus Damm, Linux I2C,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Linux-Renesas
In-Reply-To: <20190724121559.19079-2-horms+renesas@verge.net.au>
On Wed, Jul 24, 2019 at 3:25 PM Simon Horman <horms+renesas@verge.net.au> wrote:
> Rename the bindings documentation file for sh_mobile I2C controller
> from i2c-sh_mobile.txt to renesas,iic.txt.
>
> This is part of an ongoing effort to name bindings documentation files for
> Renesas IP blocks consistently, in line with the compat strings they
> document.
>
> Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH 2/4] dt-bindings: i2c: rcar: Rename bindings documentation file
From: Geert Uytterhoeven @ 2019-07-29 11:42 UTC (permalink / raw)
To: Simon Horman
Cc: Wolfram Sang, Chris Brandt, Rob Herring, Mark Rutland,
Magnus Damm, Linux I2C,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Linux-Renesas
In-Reply-To: <20190724121559.19079-3-horms+renesas@verge.net.au>
On Wed, Jul 24, 2019 at 3:25 PM Simon Horman <horms+renesas@verge.net.au> wrote:
> Rename the bindings documentation file for R-Car I2C controller
> from i2c-rcar.txt to renesas,rcar.txt.
renesas,i2c.txt
> This is part of an ongoing effort to name bindings documentation files for
> Renesas IP blocks consistently, in line with the compat strings they
> document.
>
> Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
With the above fixed:
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH 3/4] dt-bindings: i2c: riic: Rename bindings documentation file
From: Geert Uytterhoeven @ 2019-07-29 11:43 UTC (permalink / raw)
To: Simon Horman
Cc: Wolfram Sang, Chris Brandt, Rob Herring, Mark Rutland,
Magnus Damm, Linux I2C,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Linux-Renesas
In-Reply-To: <20190724121559.19079-4-horms+renesas@verge.net.au>
On Wed, Jul 24, 2019 at 3:25 PM Simon Horman <horms+renesas@verge.net.au> wrote:
> Rename the bindings documentation file for RIIC controller
> from i2c-riic.txt to renesas,riic.txt.
>
> This is part of an ongoing effort to name bindings documentation files for
> Renesas IP blocks consistently, in line with the compat strings they
> document.
>
> Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* [PATCH v3 1/2] iio: potentiometer: add a driver for Maxim 5432-5435
From: Martin Kaiser @ 2019-07-29 11:45 UTC (permalink / raw)
To: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen, Rob Herring
Cc: linux-iio, devicetree, linux-kernel, Martin Kaiser
In-Reply-To: <20190721175915.27192-1-martin@kaiser.cx>
Add a driver for the Maxim Integrated MAX5432-MAX5435 family of digital
potentiometers.
These potentiometers are connected via I2C and have 32 wiper
positions.
Supported functionality
- set the volatile wiper position
- read the potentiometer scale
Datasheet:
https://datasheets.maximintegrated.com/en/ds/MAX5432-MAX5435.pdf
Signed-off-by: Martin Kaiser <martin@kaiser.cx>
---
changes in v3
- split dt bindings and driver code into separate patches
- use yaml format for dt bindings
- fix formatting of parameter lists
changes in v2
- use MAX5432_ prefix for all defines
- fix indentation
- convert void * to unsigned long, not to u32
(warning from kbuild test robot)
drivers/iio/potentiometer/Kconfig | 11 +++
drivers/iio/potentiometer/Makefile | 1 +
drivers/iio/potentiometer/max5432.c | 135 ++++++++++++++++++++++++++++++++++++
3 files changed, 147 insertions(+)
create mode 100644 drivers/iio/potentiometer/max5432.c
diff --git a/drivers/iio/potentiometer/Kconfig b/drivers/iio/potentiometer/Kconfig
index ebc7c72a5e36..4cac0173db8b 100644
--- a/drivers/iio/potentiometer/Kconfig
+++ b/drivers/iio/potentiometer/Kconfig
@@ -26,6 +26,17 @@ config DS1803
To compile this driver as a module, choose M here: the
module will be called ds1803.
+config MAX5432
+ tristate "Maxim MAX5432-MAX5435 Digital Potentiometer driver"
+ depends on I2C
+ help
+ Say yes here to build support for the Maxim
+ MAX5432, MAX5433, MAX5434 and MAX5435 digital
+ potentiometer chips.
+
+ To compile this driver as a module, choose M here: the
+ module will be called max5432.
+
config MAX5481
tristate "Maxim MAX5481-MAX5484 Digital Potentiometer driver"
depends on SPI
diff --git a/drivers/iio/potentiometer/Makefile b/drivers/iio/potentiometer/Makefile
index 8ff55138cf12..091adf3cdd0b 100644
--- a/drivers/iio/potentiometer/Makefile
+++ b/drivers/iio/potentiometer/Makefile
@@ -6,6 +6,7 @@
# When adding new entries keep the list in alphabetical order
obj-$(CONFIG_AD5272) += ad5272.o
obj-$(CONFIG_DS1803) += ds1803.o
+obj-$(CONFIG_MAX5432) += max5432.o
obj-$(CONFIG_MAX5481) += max5481.o
obj-$(CONFIG_MAX5487) += max5487.o
obj-$(CONFIG_MCP4018) += mcp4018.o
diff --git a/drivers/iio/potentiometer/max5432.c b/drivers/iio/potentiometer/max5432.c
new file mode 100644
index 000000000000..641b1821fdf6
--- /dev/null
+++ b/drivers/iio/potentiometer/max5432.c
@@ -0,0 +1,135 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Maxim Integrated MAX5432-MAX5435 digital potentiometer driver
+ * Copyright (C) 2019 Martin Kaiser <martin@kaiser.cx>
+ *
+ * Datasheet:
+ * https://datasheets.maximintegrated.com/en/ds/MAX5432-MAX5435.pdf
+ */
+
+#include <linux/i2c.h>
+#include <linux/iio/iio.h>
+#include <linux/limits.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+
+/* All chip variants have 32 wiper positions. */
+#define MAX5432_MAX_POS 31
+
+#define MAX5432_OHM_50K (50 * 1000)
+#define MAX5432_OHM_100K (100 * 1000)
+
+/* Update the volatile (currently active) setting. */
+#define MAX5432_CMD_VREG 0x11
+
+struct max5432_data {
+ struct i2c_client *client;
+ unsigned long ohm;
+};
+
+static const struct iio_chan_spec max5432_channels[] = {
+ {
+ .type = IIO_RESISTANCE,
+ .indexed = 1,
+ .output = 1,
+ .channel = 0,
+ .address = MAX5432_CMD_VREG,
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
+ .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),
+ }
+};
+
+static int max5432_read_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ int *val, int *val2, long mask)
+{
+ struct max5432_data *data = iio_priv(indio_dev);
+
+ if (mask != IIO_CHAN_INFO_SCALE)
+ return -EINVAL;
+
+ if (unlikely(data->ohm > INT_MAX))
+ return -ERANGE;
+
+ *val = data->ohm;
+ *val2 = MAX5432_MAX_POS;
+
+ return IIO_VAL_FRACTIONAL;
+}
+
+static int max5432_write_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ int val, int val2, long mask)
+{
+ struct max5432_data *data = iio_priv(indio_dev);
+ u8 data_byte;
+
+ if (mask != IIO_CHAN_INFO_RAW)
+ return -EINVAL;
+
+ if (val < 0 || val > MAX5432_MAX_POS)
+ return -EINVAL;
+
+ if (val2 != 0)
+ return -EINVAL;
+
+ /* Wiper position is in bits D7-D3. (D2-D0 are don't care bits.) */
+ data_byte = val << 3;
+ return i2c_smbus_write_byte_data(data->client, chan->address,
+ data_byte);
+}
+
+static const struct iio_info max5432_info = {
+ .read_raw = max5432_read_raw,
+ .write_raw = max5432_write_raw,
+};
+
+static int max5432_probe(struct i2c_client *client,
+ const struct i2c_device_id *id)
+{
+ struct device *dev = &client->dev;
+ struct iio_dev *indio_dev;
+ struct max5432_data *data;
+
+ indio_dev = devm_iio_device_alloc(dev, sizeof(struct max5432_data));
+ if (!indio_dev)
+ return -ENOMEM;
+
+ i2c_set_clientdata(client, indio_dev);
+
+ data = iio_priv(indio_dev);
+ data->client = client;
+ data->ohm = (unsigned long)of_device_get_match_data(dev);
+
+ indio_dev->dev.parent = dev;
+ indio_dev->info = &max5432_info;
+ indio_dev->channels = max5432_channels;
+ indio_dev->num_channels = ARRAY_SIZE(max5432_channels);
+ indio_dev->name = client->name;
+
+ return devm_iio_device_register(dev, indio_dev);
+}
+
+static const struct of_device_id max5432_dt_ids[] = {
+ { .compatible = "maxim,max5432", .data = (void *)MAX5432_OHM_50K },
+ { .compatible = "maxim,max5433", .data = (void *)MAX5432_OHM_100K },
+ { .compatible = "maxim,max5434", .data = (void *)MAX5432_OHM_50K },
+ { .compatible = "maxim,max5435", .data = (void *)MAX5432_OHM_100K },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, max5432_dt_ids);
+
+static struct i2c_driver max5432_driver = {
+ .driver = {
+ .name = "max5432",
+ .of_match_table = of_match_ptr(max5432_dt_ids),
+ },
+ .probe = max5432_probe,
+};
+
+module_i2c_driver(max5432_driver);
+
+MODULE_AUTHOR("Martin Kaiser <martin@kaiser.cx>");
+MODULE_DESCRIPTION("max5432-max5435 digital potentiometers");
+MODULE_LICENSE("GPL v2");
--
2.11.0
^ permalink raw reply related
* [PATCH v3 2/2] dt-bindings: iio: potentiometer: add max5432.yaml binding
From: Martin Kaiser @ 2019-07-29 11:45 UTC (permalink / raw)
To: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen, Rob Herring
Cc: linux-iio, devicetree, linux-kernel, Martin Kaiser
In-Reply-To: <20190729114531.12386-1-martin@kaiser.cx>
Add a binding for the Maxim Integrated MAX5432-MAX5435 family of digital
potentiometers.
Signed-off-by: Martin Kaiser <martin@kaiser.cx>
---
changes in v3
- split dt bindings and driver code into separate patches
- use yaml format for dt bindings
- fix formatting of parameter lists
changes in v2
- use MAX5432_ prefix for all defines
- fix indentation
- convert void * to unsigned long, not to u32
(warning from kbuild test robot)
.../bindings/iio/potentiometer/max5432.yaml | 35 ++++++++++++++++++++++
1 file changed, 35 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/potentiometer/max5432.yaml
diff --git a/Documentation/devicetree/bindings/iio/potentiometer/max5432.yaml b/Documentation/devicetree/bindings/iio/potentiometer/max5432.yaml
new file mode 100644
index 000000000000..448781b80f39
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/potentiometer/max5432.yaml
@@ -0,0 +1,35 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/potentiometer/max5432.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Maxim Integrated MAX5432-MAX5435 Digital Potentiometers
+
+maintainers:
+ - Martin Kaiser <martin@kaiser.cx>
+
+description: |
+ Maxim Integrated MAX5432-MAX5435 Digital Potentiometers connected via I2C
+
+ Datasheet:
+ https://datasheets.maximintegrated.com/en/ds/MAX5432-MAX5435.pdf
+
+properties:
+ compatible:
+ enum:
+ - maxim,max5432
+ - maxim,max5433
+ - maxim,max5434
+ - maxim,max5435
+
+examples:
+ - |
+ i2c0 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ max5434@28 {
+ compatible = "maxim,max5434";
+ reg = <0x28>;
+ };
+ };
--
2.11.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox