Re: [PATCH v5 0/2] media: v4l: Add support for the Cadence MIPI-CSI2 TX controller
Hi Niklas, On Thu, Mar 01, 2018 at 05:13:38PM +0100, Niklas Söderlund wrote: > On 2018-03-01 12:30:47 +0100, Maxime Ripard wrote: > > Here is an attempt at supporting the MIPI-CSI2 TX block from Cadence. > > > > This IP block is able to receive 4 video streams and stream them over > > a MIPI-CSI2 link using up to 4 lanes. Those streams are basically the > > interfaces to controllers generating some video signals, like a camera > > or a pattern generator. > > > > It is able to map input streams to CSI2 virtual channels and datatypes > > dynamically. The streaming devices choose their virtual channels > > through an additional signal that is transparent to the CSI2-TX. The > > datatypes however are yet another additional input signal, and can be > > mapped to any CSI2 datatypes. > > > > Since v4l2 doesn't really allow for that setup at the moment, this > > preliminary version is a rather dumb one in order to start the > > discussion on how to address this properly. > > I'm sure you already are aware of this but in case you are not. Sakari > have a branch [1] which addresses much of the CSI-2 virtual channel > problems. It handles data types, virtual channels and format validation > for pipelines in IMHO good way. I have used it for my base when > implementing the R-Car CSI-2 receiver which adds a proposed way on how > to start and stop streams using Sakaris work [2]. > > Would it be possible for you to try this series on-top of Sakaris branch > and see if it fits your needs? I would be happy if it did and we can > start the process of trying to get his work upstream so we can clear > that dependency for our hopefully shared problem :-) Thanks for pointing this out :) I already started to look into this a few weeks back, and while it's not yet feature complete, it seemed to work quite well for the RX case. I haven't had time to test it on the TX controller yet. So I'd say that for now, his patches look enough to me :) Maxime -- Maxime Ripard, Bootlin (formerly Free Electrons) Embedded Linux and Kernel engineering https://bootlin.com signature.asc Description: PGP signature
[PATCH] w1: fix w1_ds2438 documentation
Signed-off-by: Mariusz Bialonczyk --- Documentation/w1/slaves/w1_ds2438 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/w1/slaves/w1_ds2438 b/Documentation/w1/slaves/w1_ds2438 index b99f3674c5b4..e64f65a09387 100644 --- a/Documentation/w1/slaves/w1_ds2438 +++ b/Documentation/w1/slaves/w1_ds2438 @@ -60,4 +60,4 @@ vad: general purpose A/D input (VAD) vdd: battery input (VDD) After the voltage conversion the value is returned as decimal ASCII. -Note: The value is in mV, so to get a volts the value has to be divided by 10. +Note: To get a volts the value has to be divided by 100. -- 2.16.2
Re: [PATCH v5 2/2] v4l: cadence: Add Cadence MIPI-CSI2 TX driver
Hi,
On Thu, Mar 01, 2018 at 05:26:58PM +0100, Niklas Söderlund wrote:
> I did not do a full review on this series, I only browsed it to check
> how you handled some CSI-2 related problems. While doing so I noticed a
> few small issues.
Thanks for your review :)
The comment I stripped out will be addressed.
> > + /*
> > +* Create a static mapping between the CSI virtual channels
> > +* and the input streams.
> > +*
> > +* This should be enhanced, but v4l2 lacks the support for
> > +* changing that mapping dynamically at the moment.
>
> Sakaris work will help with this in the kernel and I have some RFC
> patches for v4l-utils which can be used to configure it from user-space.
>
> https://www.spinics.net/lists/linux-media/msg126128.html
So my vision for this was this would come as a second step. I'd really
like to get the base driver merged, and then build on top of that to
get extra features. The mapping is definitely on my radar, and like I
was saying in my other mail, I even started to work on it.
> > +static int csi2tx_probe(struct platform_device *pdev)
> > +{
> > + struct csi2tx_priv *csi2tx;
> > + unsigned int i;
> > + int ret;
> > +
> > + csi2tx = kzalloc(sizeof(*csi2tx), GFP_KERNEL);
>
> No real issue I'm just curious, why not use devm_kzalloc() here? You
> free csi2tx in the remove() callback so not using devm_ will not help
> with (the potential) v4l2 related lifetime management issues.
Laurent asked me to do this in an earlier iteration:
https://patchwork.linuxtv.org/patch/42683/
I'm fine with both, but he seemed to say that it would be later for a
later cleanup to not use the devm variant here.
> > +MODULE_DEVICE_TABLE(of, csi2tx_of_table);
> > +
> > +static struct platform_driver csi2tx_driver = {
> > + .probe = csi2tx_probe,
> > + .remove = csi2tx_remove,
> > +
> > + .driver = {
> > + .name = "cdns-csi2tx",
> > + .of_match_table = csi2tx_of_table,
> > + },
> > +};
> > +module_platform_driver(csi2tx_driver);
>
> Are MODULE_LICENSE() not needed anymore with the introduction of the
> SPDX headers?
This is definitely missing, I'll add it.
Thanks!
Maxime
--
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
signature.asc
Description: PGP signature
Re: [PATCH v3 05/10] pwm: add PWM mode to pwm_config()
On 28.02.2018 21:44, Thierry Reding wrote: > On Thu, Feb 22, 2018 at 02:01:16PM +0200, Claudiu Beznea wrote: >> Add PWM mode to pwm_config() function. The drivers which uses pwm_config() >> were adapted to this change. >> >> Signed-off-by: Claudiu Beznea >> --- >> arch/arm/mach-s3c24xx/mach-rx1950.c | 11 +-- >> drivers/bus/ts-nbus.c| 2 +- >> drivers/clk/clk-pwm.c| 3 ++- >> drivers/gpu/drm/i915/intel_panel.c | 17 ++--- >> drivers/hwmon/pwm-fan.c | 2 +- >> drivers/input/misc/max77693-haptic.c | 2 +- >> drivers/input/misc/max8997_haptic.c | 6 +- >> drivers/leds/leds-pwm.c | 5 - >> drivers/media/rc/ir-rx51.c | 5 - >> drivers/media/rc/pwm-ir-tx.c | 5 - >> drivers/video/backlight/lm3630a_bl.c | 4 +++- >> drivers/video/backlight/lp855x_bl.c | 4 +++- >> drivers/video/backlight/lp8788_bl.c | 5 - >> drivers/video/backlight/pwm_bl.c | 11 +-- >> drivers/video/fbdev/ssd1307fb.c | 3 ++- >> include/linux/pwm.h | 6 -- >> 16 files changed, 70 insertions(+), 21 deletions(-) > > I don't think it makes sense to leak mode support into the legacy API. > The pwm_config() function is considered legacy I missed this aspect. and should eventually go > away. As such it doesn't make sense to integrate a new feature such as > PWM modes into it. Agree. All users of pwm_config() assume normal mode, and > that's what pwm_config() should provide. Agree. > > Anyone that needs something other than normal mode should use the new > atomic PWM API. Agree. > > Thierry >
Re: [PATCH v11 01/32] dt-bindings: media: rcar_vin: Reverse SoC part number list
Hi Niklas, Thank you for the patch. On Friday, 2 March 2018 03:57:20 EET Niklas Söderlund wrote: > From: Fabrizio Castro > > Change the sorting of the part numbers from descending to ascending to > match with other documentation. > > Signed-off-by: Fabrizio Castro > Reviewed-by: Biju Das > Reviewed-by: Simon Horman > Acked-by: Rob Herring > Reviewed-by: Geert Uytterhoeven > Acked-by: Niklas Söderlund Reviewed-by: Laurent Pinchart > --- > Documentation/devicetree/bindings/media/rcar_vin.txt | 14 +++--- > 1 file changed, 7 insertions(+), 7 deletions(-) > > diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt > b/Documentation/devicetree/bindings/media/rcar_vin.txt index > 19357d0bbe6539b3..0ac715a5c331bc26 100644 > --- a/Documentation/devicetree/bindings/media/rcar_vin.txt > +++ b/Documentation/devicetree/bindings/media/rcar_vin.txt > @@ -6,14 +6,14 @@ family of devices. The current blocks are always slaves > and suppot one input channel which can be either RGB, YUYV or BT656. > > - compatible: Must be one or more of the following > - - "renesas,vin-r8a7795" for the R8A7795 device > - - "renesas,vin-r8a7794" for the R8A7794 device > - - "renesas,vin-r8a7793" for the R8A7793 device > - - "renesas,vin-r8a7792" for the R8A7792 device > - - "renesas,vin-r8a7791" for the R8A7791 device > - - "renesas,vin-r8a7790" for the R8A7790 device > - - "renesas,vin-r8a7779" for the R8A7779 device > - "renesas,vin-r8a7778" for the R8A7778 device > + - "renesas,vin-r8a7779" for the R8A7779 device > + - "renesas,vin-r8a7790" for the R8A7790 device > + - "renesas,vin-r8a7791" for the R8A7791 device > + - "renesas,vin-r8a7792" for the R8A7792 device > + - "renesas,vin-r8a7793" for the R8A7793 device > + - "renesas,vin-r8a7794" for the R8A7794 device > + - "renesas,vin-r8a7795" for the R8A7795 device > - "renesas,rcar-gen2-vin" for a generic R-Car Gen2 compatible device. > - "renesas,rcar-gen3-vin" for a generic R-Car Gen3 compatible device. -- Regards, Laurent Pinchart
Re: [PATCH v11 02/32] dt-bindings: media: rcar_vin: add device tree support for r8a774[35]
Hi Niklas, Thank you for the patch. On Friday, 2 March 2018 03:57:21 EET Niklas Söderlund wrote: > From: Fabrizio Castro > > Add compatible strings for r8a7743 and r8a7745. No driver change > is needed as "renesas,rcar-gen2-vin" will activate the right code. > However, it is good practice to document compatible strings for the > specific SoC as this allows SoC specific changes to the driver if > needed, in addition to document SoC support and therefore allow > checkpatch.pl to validate compatible string values. > > Signed-off-by: Fabrizio Castro > Reviewed-by: Biju Das > Reviewed-by: Simon Horman > Acked-by: Rob Herring > Reviewed-by: Geert Uytterhoeven > Acked-by: Niklas Söderlund Reviewed-by: Laurent Pinchart > --- > Documentation/devicetree/bindings/media/rcar_vin.txt | 5 - > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt > b/Documentation/devicetree/bindings/media/rcar_vin.txt index > 0ac715a5c331bc26..c60e6b0a89b67a8c 100644 > --- a/Documentation/devicetree/bindings/media/rcar_vin.txt > +++ b/Documentation/devicetree/bindings/media/rcar_vin.txt > @@ -6,6 +6,8 @@ family of devices. The current blocks are always slaves and > suppot one input channel which can be either RGB, YUYV or BT656. > > - compatible: Must be one or more of the following > + - "renesas,vin-r8a7743" for the R8A7743 device > + - "renesas,vin-r8a7745" for the R8A7745 device > - "renesas,vin-r8a7778" for the R8A7778 device > - "renesas,vin-r8a7779" for the R8A7779 device > - "renesas,vin-r8a7790" for the R8A7790 device > @@ -14,7 +16,8 @@ channel which can be either RGB, YUYV or BT656. > - "renesas,vin-r8a7793" for the R8A7793 device > - "renesas,vin-r8a7794" for the R8A7794 device > - "renesas,vin-r8a7795" for the R8A7795 device > - - "renesas,rcar-gen2-vin" for a generic R-Car Gen2 compatible device. > + - "renesas,rcar-gen2-vin" for a generic R-Car Gen2 or RZ/G1 compatible > + device. > - "renesas,rcar-gen3-vin" for a generic R-Car Gen3 compatible device. > > When compatible with the generic version nodes must list the -- Regards, Laurent Pinchart
Re: [PATCH v3 05/10] pwm: add PWM mode to pwm_config()
On 28.02.2018 22:04, Jani Nikula wrote:
> On Wed, 28 Feb 2018, Thierry Reding wrote:
>> Anyone that needs something other than normal mode should use the new
>> atomic PWM API.
>
> At the risk of revealing my true ignorance, what is the new atomic PWM
> API? Where? Examples of how one would convert old code over to the new
> API?
As far as I know, the old PWM core code uses config(), set_polarity(),
enable(), disable() methods of driver, registered as pwm_ops:
struct pwm_ops {
int (*request)(struct pwm_chip *chip, struct pwm_device *pwm);
void (*free)(struct pwm_chip *chip, struct pwm_device *pwm);
int (*config)(struct pwm_chip *chip, struct pwm_device *pwm,
int duty_ns, int period_ns);
int (*set_polarity)(struct pwm_chip *chip, struct pwm_device *pwm,
enum pwm_polarity polarity);
int (*capture)(struct pwm_chip *chip, struct pwm_device *pwm,
struct pwm_capture *result, unsigned long timeout);
int (*enable)(struct pwm_chip *chip, struct pwm_device *pwm);
void (*disable)(struct pwm_chip *chip, struct pwm_device *pwm);
int (*apply)(struct pwm_chip *chip, struct pwm_device *pwm,
struct pwm_state *state);
void (*get_state)(struct pwm_chip *chip, struct pwm_device *pwm,
struct pwm_state *state);
#ifdef CONFIG_DEBUG_FS
void (*dbg_show)(struct pwm_chip *chip, struct seq_file *s);
#endif
struct module *owner;
};
to do settings on hardware. In order to so settings on a PWM the users
should have been follow the below steps:
->config()
->set_polarity()
->enable()
Moreover, if the PWM was previously enabled it should have been first
disable and then to follow the above steps in order to apply a new settings
on hardware.
The driver should have been provide, at probe, all the above function:
->config(), ->set_polarity(), ->disable(), ->enable(), function that were
used by PWM core.
Now, having atomic PWM, the driver should provide one function to PWM core,
which is ->apply() function. Every PWM has a state associated, which keeps
the period, duty cycle, polarity and enable/disable status. The driver's
->apply() function takes as argument the state that should be applied and
it takes care of applying this new state directly without asking user to
call ->disable(), then ->config()/->set_polarity(), then ->enable() to
apply new hardware settings.
The PWM consumer could set a new state for PWM it uses, using
pwm_apply_state(pwm, new_state);
Regarding the models to switch on atomic PWM, on the controller side you
can check for drivers that registers apply function at probe time.
Regarding the PWM users, you can look for pwm_apply_state()
(drivers/hwmon/pwm-fan.c or drivers/input/misc/pwm-beeper.c are some examples).
Thierry, please correct me if I'm wrong.
Thank you,
Claudiu Beznea
>
> BR,
> Jani.
>
Re: [PATCH v11 11/32] rcar-vin: set a default field to fallback on
Hi Niklas,
Thank you for the patch.
On Friday, 2 March 2018 03:57:30 EET Niklas Söderlund wrote:
> If the field is not supported by the driver it should not try to keep
> the current field. Instead it should set it to a default fallback. Since
> trying a format should always result in the same state regardless of the
> current state of the device.
>
> Signed-off-by: Niklas Söderlund
Reviewed-by: Laurent Pinchart
> ---
> drivers/media/platform/rcar-vin/rcar-v4l2.c | 9 +++--
> 1 file changed, 3 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c
> b/drivers/media/platform/rcar-vin/rcar-v4l2.c index
> c2265324c7c96308..ebcd78b1bb6e8cb6 100644
> --- a/drivers/media/platform/rcar-vin/rcar-v4l2.c
> +++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c
> @@ -23,6 +23,7 @@
> #include "rcar-vin.h"
>
> #define RVIN_DEFAULT_FORMAT V4L2_PIX_FMT_YUYV
> +#define RVIN_DEFAULT_FIELD V4L2_FIELD_NONE
>
> /*
> ---
> -- * Format Conversions
> @@ -143,7 +144,7 @@ static int rvin_reset_format(struct rvin_dev *vin)
> case V4L2_FIELD_INTERLACED:
> break;
> default:
> - vin->format.field = V4L2_FIELD_NONE;
> + vin->format.field = RVIN_DEFAULT_FIELD;
> break;
> }
>
> @@ -213,10 +214,6 @@ static int __rvin_try_format(struct rvin_dev *vin,
> u32 walign;
> int ret;
>
> - /* Keep current field if no specific one is asked for */
> - if (pix->field == V4L2_FIELD_ANY)
> - pix->field = vin->format.field;
> -
> /* If requested format is not supported fallback to the default */
> if (!rvin_format_from_pixel(pix->pixelformat)) {
> vin_dbg(vin, "Format 0x%x not found, using default 0x%x\n",
> @@ -246,7 +243,7 @@ static int __rvin_try_format(struct rvin_dev *vin,
> case V4L2_FIELD_INTERLACED:
> break;
> default:
> - pix->field = V4L2_FIELD_NONE;
> + pix->field = RVIN_DEFAULT_FIELD;
> break;
> }
--
Regards,
Laurent Pinchart
Your response is important
Hello, I am contacting you to be my foreign partner in a financial transaction in my Corporation with the objective of investing the fund in your country. Thanks in anticipation of your response. Mr.Jeddy
[GIT FIXES FOR v4.16] tegra-cec: reset rx_buf_cnt when start bit detected
Fix for 4.16 with CC to stable for 4.15. Regards, Hans The following changes since commit e3e389f931a14ddf43089c7db92fc5d74edf93a4: media: rc: fix race condition in ir_raw_event_store_edge() handling (2018-02-27 08:16:09 -0500) are available in the git repository at: git://linuxtv.org/hverkuil/media_tree.git for-v4.16g for you to fetch changes up to 21bc63372793a0e66d46f95cd9c395204a80fd24: tegra-cec: reset rx_buf_cnt when start bit detected (2018-03-02 10:33:18 +0100) Hans Verkuil (1): tegra-cec: reset rx_buf_cnt when start bit detected drivers/media/platform/tegra-cec/tegra_cec.c | 17 +++-- 1 file changed, 7 insertions(+), 10 deletions(-)
Re: [PATCH v8 2/2] v4l: cadence: Add Cadence MIPI-CSI2 RX driver
Hi Benoit,
On Thu, Mar 01, 2018 at 02:09:18PM -0600, Benoit Parrot wrote:
> > + /*
> > +* FIXME: Once we'll have internal D-PHY support, the check
> > +* will need to be removed.
> > +*/
> > + if (csi2rx->has_internal_dphy) {
> > + dev_err(&pdev->dev, "Internal D-PHY not supported yet\n");
> > + return -EINVAL;
> > + }
>
> As one of the more critical thing is usually how the CSI2 Receiver interact
> with a DPHY when can we expect this part of the driver to be implemented?
That's definitely on my radar, but it isn't implemented by any
hardware or simulation at the moment. When that is done, we will
obviously do it.
Maxime
--
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
signature.asc
Description: PGP signature
[PATCH] media: doc: poll: fix links to dual-ioctl sections
Links like :ref:`VIDIOC_STREAMON` expand to "ioctl VIDIOC_STREAMON, VIDIOC_STREAMOFF". Thus our reader will think we are talking about STREAMON _and_ STREAMOFF, but only one of the two actually applies in some cases. Fix by adding a link title, so the reader will read only the correct ioctl name. Signed-off-by: Luca Ceresoli Cc: Mauro Carvalho Chehab --- Documentation/media/uapi/v4l/func-poll.rst | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Documentation/media/uapi/v4l/func-poll.rst b/Documentation/media/uapi/v4l/func-poll.rst index d0432dc09b05..360bc6523ae2 100644 --- a/Documentation/media/uapi/v4l/func-poll.rst +++ b/Documentation/media/uapi/v4l/func-poll.rst @@ -39,7 +39,7 @@ When streaming I/O has been negotiated this function waits until a buffer has been filled by the capture device and can be dequeued with the :ref:`VIDIOC_DQBUF ` ioctl. For output devices this function waits until the device is ready to accept a new buffer to be -queued up with the :ref:`VIDIOC_QBUF` ioctl for +queued up with the :ref:`VIDIOC_QBUF ` ioctl for display. When buffers are already in the outgoing queue of the driver (capture) or the incoming queue isn't full (display) the function returns immediately. @@ -52,11 +52,11 @@ flags in the ``revents`` field, output devices the ``POLLOUT`` and ``POLLWRNORM`` flags. When the function timed out it returns a value of zero, on failure it returns -1 and the ``errno`` variable is set appropriately. When the application did not call -:ref:`VIDIOC_STREAMON` the :ref:`poll() ` +:ref:`VIDIOC_STREAMON ` the :ref:`poll() ` function succeeds, but sets the ``POLLERR`` flag in the ``revents`` field. When the application has called -:ref:`VIDIOC_STREAMON` for a capture device but -hasn't yet called :ref:`VIDIOC_QBUF`, the +:ref:`VIDIOC_STREAMON ` for a capture device but +hasn't yet called :ref:`VIDIOC_QBUF `, the :ref:`poll() ` function succeeds and sets the ``POLLERR`` flag in the ``revents`` field. For output devices this same situation will cause :ref:`poll() ` to succeed as well, but it sets the ``POLLOUT`` and -- 2.7.4
Re: [PATCH v11 13/32] rcar-vin: update bytesperline and sizeimage calculation
Hi Niklas,
Thank you for the patch.
On Friday, 2 March 2018 03:57:32 EET Niklas Söderlund wrote:
> Remove over complicated logic to calculate the value for bytesperline
> and sizeimage that was carried over from the soc_camera port. There is
> no need to find the max value of bytesperline and sizeimage from
> user-space as they are set to 0 before the max_t() operation.
>
> Signed-off-by: Niklas Söderlund
Reviewed-by: Laurent Pinchart
> ---
> drivers/media/platform/rcar-vin/rcar-v4l2.c | 10 ++
> 1 file changed, 2 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c
> b/drivers/media/platform/rcar-vin/rcar-v4l2.c index
> cef9070884d93ba6..652b85300b4ef9db 100644
> --- a/drivers/media/platform/rcar-vin/rcar-v4l2.c
> +++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c
> @@ -194,10 +194,6 @@ static int __rvin_try_format(struct rvin_dev *vin,
> pix->pixelformat = RVIN_DEFAULT_FORMAT;
> }
>
> - /* Always recalculate */
> - pix->bytesperline = 0;
> - pix->sizeimage = 0;
> -
> /* Limit to source capabilities */
> ret = __rvin_try_format_source(vin, which, pix, source);
> if (ret)
> @@ -232,10 +228,8 @@ static int __rvin_try_format(struct rvin_dev *vin,
> v4l_bound_align_image(&pix->width, 2, vin->info->max_width, walign,
> &pix->height, 4, vin->info->max_height, 2, 0);
>
> - pix->bytesperline = max_t(u32, pix->bytesperline,
> - rvin_format_bytesperline(pix));
> - pix->sizeimage = max_t(u32, pix->sizeimage,
> -rvin_format_sizeimage(pix));
> + pix->bytesperline = rvin_format_bytesperline(pix);
> + pix->sizeimage = rvin_format_sizeimage(pix);
>
> if (vin->info->model == RCAR_M1 &&
> pix->pixelformat == V4L2_PIX_FMT_XBGR32) {
--
Regards,
Laurent Pinchart
Re: [PATCH v5 2/2] v4l: cadence: Add Cadence MIPI-CSI2 TX driver
Hi Benoit, On Thu, Mar 01, 2018 at 02:35:16PM -0600, Benoit Parrot wrote: > > + writel(CSI2TX_DPHY_CLK_WAKEUP_ULPS_CYCLES(32), > > + csi2tx->base + CSI2TX_DPHY_CLK_WAKEUP_REG); > > I am sorry if I missed this previously but do all these > CSI2TX_DPHY* reg access assume that "has_internal_dphy" is true? It seems like it's needed even for the case without an internal DPHY. I'll check, and add a comment if it's really the case. Thanks! Maxime -- Maxime Ripard, Bootlin (formerly Free Electrons) Embedded Linux and Kernel engineering https://bootlin.com signature.asc Description: PGP signature
Re: [PATCH v11 15/32] rcar-vin: break out format alignment and checking
Hi Niklas,
Thank you for the patch.
On Friday, 2 March 2018 03:57:34 EET Niklas Söderlund wrote:
> Part of the format alignment and checking can be shared with the Gen3
> format handling. Break that part out to a separate function.
>
> Signed-off-by: Niklas Söderlund
> ---
> drivers/media/platform/rcar-vin/rcar-v4l2.c | 96 +++---
> 1 file changed, 54 insertions(+), 42 deletions(-)
>
> diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c
> b/drivers/media/platform/rcar-vin/rcar-v4l2.c index
> b94ca9ffb1d3b323..3290e603b44cdf3a 100644
> --- a/drivers/media/platform/rcar-vin/rcar-v4l2.c
> +++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c
> @@ -87,6 +87,59 @@ static u32 rvin_format_sizeimage(struct v4l2_pix_format
> *pix) return pix->bytesperline * pix->height;
> }
>
> +static int rvin_format_align(struct rvin_dev *vin, struct v4l2_pix_format
> *pix)
> +{
> + u32 walign;
> +
> + if (!rvin_format_from_pixel(pix->pixelformat) ||
> + (vin->info->model == RCAR_M1 &&
> + pix->pixelformat == V4L2_PIX_FMT_XBGR32))
> + pix->pixelformat = RVIN_DEFAULT_FORMAT;
> +
> + switch (pix->field) {
> + case V4L2_FIELD_TOP:
> + case V4L2_FIELD_BOTTOM:
> + case V4L2_FIELD_NONE:
> + case V4L2_FIELD_INTERLACED_TB:
> + case V4L2_FIELD_INTERLACED_BT:
> + case V4L2_FIELD_INTERLACED:
> + break;
> + case V4L2_FIELD_ALTERNATE:
> + /*
> + * Driver do not (yet) support outputting ALTERNATE to a
> + * userspace. It does support outputting INTERLACED so use
> + * the VIN hardware to combine the two fields.
> + */
> + pix->field = V4L2_FIELD_INTERLACED;
> + pix->height *= 2;
> + break;
> + default:
> + pix->field = RVIN_DEFAULT_FIELD;
> + break;
> + }
> +
> + /* HW limit width to a multiple of 32 (2^5) for NV16 else 2 (2^1) */
> + walign = vin->format.pixelformat == V4L2_PIX_FMT_NV16 ? 5 : 1;
> +
> + /* Limit to VIN capabilities */
> + v4l_bound_align_image(&pix->width, 2, vin->info->max_width, walign,
> + &pix->height, 4, vin->info->max_height, 2, 0);
> +
> + pix->bytesperline = rvin_format_bytesperline(pix);
> + pix->sizeimage = rvin_format_sizeimage(pix);
> +
> + if (vin->info->model == RCAR_M1 &&
> + pix->pixelformat == V4L2_PIX_FMT_XBGR32) {
> + vin_err(vin, "pixel format XBGR32 not supported on M1\n");
> + return -EINVAL;
> + }
This can't happen as you set the pixel format to the default earlier in the
same case.
> + vin_dbg(vin, "Format %ux%u bpl: %d size: %d\n",
> + pix->width, pix->height, pix->bytesperline, pix->sizeimage);
While at it you could use %u for bpl and size.
> +
> + return 0;
> +}
> +
> /*
> * V4L2
> */
> @@ -184,55 +237,14 @@ static int __rvin_try_format(struct rvin_dev *vin,
>struct v4l2_pix_format *pix,
>struct rvin_source_fmt *source)
> {
> - u32 walign;
> int ret;
>
> - if (!rvin_format_from_pixel(pix->pixelformat) ||
> - (vin->info->model == RCAR_M1 &&
> - pix->pixelformat == V4L2_PIX_FMT_XBGR32))
> - pix->pixelformat = RVIN_DEFAULT_FORMAT;
> -
> /* Limit to source capabilities */
> ret = __rvin_try_format_source(vin, which, pix, source);
This functions uses the pixel format. Isn't it a problem that you don't first
set it to the default if the requested pixel format isn't supported ?
> if (ret)
> return ret;
>
> - switch (pix->field) {
> - case V4L2_FIELD_TOP:
> - case V4L2_FIELD_BOTTOM:
> - case V4L2_FIELD_NONE:
> - case V4L2_FIELD_INTERLACED_TB:
> - case V4L2_FIELD_INTERLACED_BT:
> - case V4L2_FIELD_INTERLACED:
> - break;
> - case V4L2_FIELD_ALTERNATE:
> - /*
> - * Driver do not (yet) support outputting ALTERNATE to a
> - * userspace. It does support outputting INTERLACED so use
> - * the VIN hardware to combine the two fields.
> - */
> - pix->field = V4L2_FIELD_INTERLACED;
> - pix->height *= 2;
> - break;
> - default:
> - pix->field = RVIN_DEFAULT_FIELD;
> - break;
> - }
> -
> - /* HW limit width to a multiple of 32 (2^5) for NV16 else 2 (2^1) */
> - walign = vin->format.pixelformat == V4L2_PIX_FMT_NV16 ? 5 : 1;
> -
> - /* Limit to VIN capabilities */
> - v4l_bound_align_image(&pix->width, 2, vin->info->max_width, walign,
> - &pix->height, 4, vin->info->max_height, 2, 0);
> -
> - pix->bytesperline = rvin_format_bytesperline(pix);
> - pix->sizeimage = rvin_format_sizeimage(pix);
> -
> - vin_dbg(vin, "Format %ux%u bpl: %d si
Re: [PATCH v11 22/32] rcar-vin: force default colorspace for media centric mode
Hi Niklas,
Thank you for the patch.
On Friday, 2 March 2018 03:57:41 EET Niklas Söderlund wrote:
> When the VIN driver is running in media centric mode (on Gen3) the
> colorspace is not retrieved from the video source instead the user is
> expected to set it as part of the format. There is no way for the VIN
> driver to validated the colorspace requested by user-space, this creates
> a problem where validation tools fail. Until the user requested
> colorspace can be validated lets force it to the driver default.
The problem here is that the V4L2 specification clearly documents the
colorspace fields as being set by drivers for capture devices. Using the
values supplied by userspace thus wouldn't comply with the API. The API has to
be updated to allow us to implement this feature, but until then Hans wants
the userspace to be set by the driver to a fixed value. Could you update the
commit message accordingly, as well as the comment below ?
> Signed-off-by: Niklas Söderlund
> ---
> drivers/media/platform/rcar-vin/rcar-v4l2.c | 16 ++--
> 1 file changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c
> b/drivers/media/platform/rcar-vin/rcar-v4l2.c index
> 8d92710efffa7276..02f3100ed30db63c 100644
> --- a/drivers/media/platform/rcar-vin/rcar-v4l2.c
> +++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c
> @@ -675,12 +675,24 @@ static const struct v4l2_ioctl_ops rvin_ioctl_ops = {
> * V4L2 Media Controller
> */
>
> +static int rvin_mc_try_format(struct rvin_dev *vin, struct v4l2_pix_format
> *pix) +{
> + /*
> + * There is no way to validate the colorspace provided by the
> + * user. Until it can be validated force colorspace to the
> + * driver default.
> + */
> + pix->colorspace = RVIN_DEFAULT_COLORSPACE;
Should you also set the xfer_func, quantization and ycbcr_enc ?
Apart from that,
Reviewed-by: Laurent Pinchart
> +
> + return rvin_format_align(vin, pix);
> +}
> +
> static int rvin_mc_try_fmt_vid_cap(struct file *file, void *priv,
> struct v4l2_format *f)
> {
> struct rvin_dev *vin = video_drvdata(file);
>
> - return rvin_format_align(vin, &f->fmt.pix);
> + return rvin_mc_try_format(vin, &f->fmt.pix);
> }
>
> static int rvin_mc_s_fmt_vid_cap(struct file *file, void *priv,
> @@ -692,7 +704,7 @@ static int rvin_mc_s_fmt_vid_cap(struct file *file, void
> *priv, if (vb2_is_busy(&vin->queue))
> return -EBUSY;
>
> - ret = rvin_format_align(vin, &f->fmt.pix);
> + ret = rvin_mc_try_format(vin, &f->fmt.pix);
> if (ret)
> return ret;
--
Regards,
Laurent Pinchart
Re: [PATCH v11 12/32] rcar-vin: fix handling of single field frames (top, bottom and alternate fields)
Hi Niklas,
Thank you for the patch.
On Friday, 2 March 2018 03:57:31 EET Niklas Söderlund wrote:
> There was never proper support in the VIN driver to deliver ALTERNATING
> field format to user-space, remove this field option. The problem is
> that ALTERNATING filed order requires the sequence numbers of buffers
s/filed/field/
> returned to userspace to reflect if fields where dropped or not,
> something which is not possible with the VIN drivers capture logic.
>
> The VIN driver can still capture from a video source which delivers
> frames in ALTERNATING field order, but needs to combine them using the
> VIN hardware into INTERLACED field order. Before this change if a source
> was delivering fields using ALTERNATE the driver would default to
> combining them using this hardware feature. Only if the user explicitly
> requested ALTERNATE filed order would incorrect frames be delivered.
>
> The height should not be cut in half for the format for TOP or BOTTOM
> fields settings. This was a mistake and it was made visible by the
> scaling refactoring. Correct behavior is that the user should request a
> frame size that fits the half height frame reflected in the field
> setting. If not the VIN will do its best to scale the top or bottom to
> the requested format and cropping and scaling do not work as expected.
>
> Signed-off-by: Niklas Söderlund
> ---
> drivers/media/platform/rcar-vin/rcar-dma.c | 15 +--
> drivers/media/platform/rcar-vin/rcar-v4l2.c | 40 +++---
> 2 files changed, 10 insertions(+), 45 deletions(-)
>
> diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c
> b/drivers/media/platform/rcar-vin/rcar-dma.c index
> fd14be20a6604d7a..c8831e189d362c8b 100644
> --- a/drivers/media/platform/rcar-vin/rcar-dma.c
> +++ b/drivers/media/platform/rcar-vin/rcar-dma.c
> @@ -617,7 +617,6 @@ static int rvin_setup(struct rvin_dev *vin)
> case V4L2_FIELD_INTERLACED_BT:
> vnmc = VNMC_IM_FULL | VNMC_FOC;
> break;
> - case V4L2_FIELD_ALTERNATE:
Just to make sure I understand things correctly, field will never be
V4L2_FIELD_ALTERNATE after this patch, right ?
> case V4L2_FIELD_NONE:
> if (vin->continuous) {
> vnmc = VNMC_IM_ODD_EVEN;
> @@ -757,18 +756,6 @@ static int rvin_get_active_slot(struct rvin_dev *vin,
> u32 vnms) return 0;
> }
>
> -static enum v4l2_field rvin_get_active_field(struct rvin_dev *vin, u32
> vnms)
> -{
> - if (vin->format.field == V4L2_FIELD_ALTERNATE) {
> - /* If FS is set it's a Even field */
> - if (vnms & VNMS_FS)
> - return V4L2_FIELD_BOTTOM;
> - return V4L2_FIELD_TOP;
> - }
> -
> - return vin->format.field;
> -}
> -
> static void rvin_set_slot_addr(struct rvin_dev *vin, int slot, dma_addr_t
> addr) {
> const struct rvin_video_format *fmt;
> @@ -941,7 +928,7 @@ static irqreturn_t rvin_irq(int irq, void *data)
> goto done;
>
> /* Capture frame */
> - vin->queue_buf[slot]->field = rvin_get_active_field(vin, vnms);
> + vin->queue_buf[slot]->field = vin->format.field;
> vin->queue_buf[slot]->sequence = sequence;
> vin->queue_buf[slot]->vb2_buf.timestamp = ktime_get_ns();
> vb2_buffer_done(&vin->queue_buf[slot]->vb2_buf, VB2_BUF_STATE_DONE);
> diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c
> b/drivers/media/platform/rcar-vin/rcar-v4l2.c index
> ebcd78b1bb6e8cb6..cef9070884d93ba6 100644
> --- a/drivers/media/platform/rcar-vin/rcar-v4l2.c
> +++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c
> @@ -121,33 +121,6 @@ static int rvin_reset_format(struct rvin_dev *vin)
> vin->format.colorspace = mf->colorspace;
> vin->format.field = mf->field;
>
> - /*
> - * If the subdevice uses ALTERNATE field mode and G_STD is
> - * implemented use the VIN HW to combine the two fields to
> - * one INTERLACED frame. The ALTERNATE field mode can still
> - * be requested in S_FMT and be respected, this is just the
> - * default which is applied at probing or when S_STD is called.
> - */
> - if (vin->format.field == V4L2_FIELD_ALTERNATE &&
> - v4l2_subdev_has_op(vin_to_source(vin), video, g_std))
> - vin->format.field = V4L2_FIELD_INTERLACED;
> -
> - switch (vin->format.field) {
> - case V4L2_FIELD_TOP:
> - case V4L2_FIELD_BOTTOM:
> - case V4L2_FIELD_ALTERNATE:
> - vin->format.height /= 2;
> - break;
> - case V4L2_FIELD_NONE:
> - case V4L2_FIELD_INTERLACED_TB:
> - case V4L2_FIELD_INTERLACED_BT:
> - case V4L2_FIELD_INTERLACED:
> - break;
> - default:
> - vin->format.field = RVIN_DEFAULT_FIELD;
> - break;
> - }
> -
I might be wrong, but if this function is called from S_STD or S_DV_TIMINGS
and userspace calls G_FMT immediately after that, won't it get
V4L2_FIELD_ALTERNATE (assuming that's what t
Re: [PATCH] staging/imx: Fix inconsistent IS_ERR and PTR_ERR
Hi Fabio,
On Thu, 2018-03-01 at 13:43 -0300, Fabio Estevam wrote:
> On Thu, Mar 1, 2018 at 1:27 PM, Philipp Zabel wrote:
>
> > Oh, this only works for csi ports that have pinctrl in their csi port
> > node, like:
> >
> > &ipu1_csi0 {
> > pinctrl-names = "default";
> > pinctrl-0 = <&pinctrl_ipu1_csi0>;
> > };
>
> This is the case for imx6qdl-sabresd.dtsi and even in this case
> devm_pinctrl_get_select_default() fails
>
> > pinctrl would have to be moved out of the csi port nodes, for example
> > into their parent ipu nodes, or maybe more correctly, into the video mux
> > nodes in each device tree.
>
> Tried it like this:
>
> --- a/arch/arm/boot/dts/imx6qdl-sabresd.dtsi
> +++ b/arch/arm/boot/dts/imx6qdl-sabresd.dtsi
> @@ -154,12 +154,9 @@
> };
>
> &ipu1_csi0_mux_from_parallel_sensor {
> - remote-endpoint = <&ov5642_to_ipu1_csi0_mux>;
> -};
> -
> -&ipu1_csi0 {
> pinctrl-names = "default";
> pinctrl-0 = <&pinctrl_ipu1_csi0>;
> + remote-endpoint = <&ov5642_to_ipu1_csi0_mux>;
> };
>
> &mipi_csi {
>
>
> but still get the devm_pinctrl_get_select_default() failure.
Yes, this would still require to ignore the pinctrl error in the CSI
driver.
I just think that this is might be more correct, since the external pins
are directly connected to the mux input port.
> I was not able to change the dts so that
> devm_pinctrl_get_select_default() succeeds.
>
> If you agree I can send the following change:
>
> diff --git a/drivers/staging/media/imx/imx-media-csi.c
> b/drivers/staging/media/imx/imx-media-csi.c
> index 5a195f8..c40f786 100644
> --- a/drivers/staging/media/imx/imx-media-csi.c
> +++ b/drivers/staging/media/imx/imx-media-csi.c
> @@ -1797,11 +1797,8 @@ static int imx_csi_probe(struct platform_device *pdev)
> */
> priv->dev->of_node = pdata->of_node;
> pinctrl = devm_pinctrl_get_select_default(priv->dev);
> - if (IS_ERR(pinctrl)) {
> - ret = PTR_ERR(priv->vdev);
> - goto free;
> - }
> -
> + if (IS_ERR(pinctrl))
> + dev_dbg(priv->dev, "pintrl_get_select_default() failed\n");
I would add the error code to the debug print.
> ret = v4l2_async_register_subdev(&priv->sd);
> if (ret)
> goto free;
>
> So that the error is ignored and we still can change the pinctrl values via
> dts.
>
> What do you think?
Maybe we should still throw the error if it is anything other than
-ENODEV (which we expect in case there is no pinctrl property in the csi
port node):
if (IS_ERR(pinctrl)) {
ret = PTR_ERR(pinctrl);
dev_dbg(priv->dev, "pinctrl_get_select_default() failed: %d\n",
ret);
if (ret != -ENODEV)
goto free;
}
regards
Philipp
Re: [PATCH v11 16/32] rcar-vin: read subdevice format for crop only when needed
Hi Niklas,
Thank you for the patch.
On Friday, 2 March 2018 03:57:35 EET Niklas Söderlund wrote:
> Instead of caching the subdevice format each time the video device
> format is set read it directly when it's needed. As it turns out the
> format is only needed when figuring out the max rectangle for cropping.
>
> This simplifies the code and makes it clearer what the source format is
> used for.
>
> Signed-off-by: Niklas Söderlund
> ---
> drivers/media/platform/rcar-vin/rcar-v4l2.c | 158 +
> drivers/media/platform/rcar-vin/rcar-vin.h | 12 ---
> 2 files changed, 80 insertions(+), 90 deletions(-)
>
> diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c
> b/drivers/media/platform/rcar-vin/rcar-v4l2.c index
> 3290e603b44cdf3a..55640c6b2a1200ca 100644
> --- a/drivers/media/platform/rcar-vin/rcar-v4l2.c
> +++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c
> @@ -144,67 +144,62 @@ static int rvin_format_align(struct rvin_dev *vin,
> struct v4l2_pix_format *pix) * V4L2
> */
>
> -static void rvin_reset_crop_compose(struct rvin_dev *vin)
> +static int rvin_get_vin_format_from_source(struct rvin_dev *vin,
> +struct v4l2_pix_format *pix)
> {
> + struct v4l2_subdev_format fmt = {
> + .which = V4L2_SUBDEV_FORMAT_ACTIVE,
> + .pad = vin->digital->source_pad,
> + };
> + int ret;
> +
> + ret = v4l2_subdev_call(vin_to_source(vin), pad, get_fmt, NULL, &fmt);
> + if (ret)
> + return ret;
> +
> + v4l2_fill_pix_format(pix, &fmt.format);
> +
> + return rvin_format_align(vin, pix);
> +}
> +
> +static int rvin_reset_format(struct rvin_dev *vin)
> +{
> + int ret;
> +
> + ret = rvin_get_vin_format_from_source(vin, &vin->format);
> + if (ret)
> + return ret;
> +
> vin->crop.top = vin->crop.left = 0;
> - vin->crop.width = vin->source.width;
> - vin->crop.height = vin->source.height;
> + vin->crop.width = vin->format.width;
> + vin->crop.height = vin->format.height;
>
> vin->compose.top = vin->compose.left = 0;
> vin->compose.width = vin->format.width;
> vin->compose.height = vin->format.height;
> -}
> -
> -static int rvin_reset_format(struct rvin_dev *vin)
> -{
> - struct v4l2_subdev_format fmt = {
> - .which = V4L2_SUBDEV_FORMAT_ACTIVE,
> - };
> - struct v4l2_mbus_framefmt *mf = &fmt.format;
> - int ret;
> -
> - fmt.pad = vin->digital->source_pad;
> -
> - ret = v4l2_subdev_call(vin_to_source(vin), pad, get_fmt, NULL, &fmt);
> - if (ret)
> - return ret;
> -
> - vin->format.width = mf->width;
> - vin->format.height = mf->height;
> - vin->format.colorspace = mf->colorspace;
> - vin->format.field = mf->field;
> -
> - rvin_reset_crop_compose(vin);
> -
> - vin->format.bytesperline = rvin_format_bytesperline(&vin->format);
> - vin->format.sizeimage = rvin_format_sizeimage(&vin->format);
>
> return 0;
> }
>
> -static int __rvin_try_format_source(struct rvin_dev *vin,
> - u32 which,
> - struct v4l2_pix_format *pix,
> - struct rvin_source_fmt *source)
> +static int rvin_try_format(struct rvin_dev *vin, u32 which,
> +struct v4l2_pix_format *pix,
> +struct v4l2_rect *crop, struct v4l2_rect *compose)
> {
> - struct v4l2_subdev *sd;
> + struct v4l2_subdev *sd = vin_to_source(vin);
> struct v4l2_subdev_pad_config *pad_cfg;
> struct v4l2_subdev_format format = {
> .which = which,
> + .pad = vin->digital->source_pad,
> };
> enum v4l2_field field;
> u32 width, height;
> int ret;
>
> - sd = vin_to_source(vin);
> -
> - v4l2_fill_mbus_format(&format.format, pix, vin->digital->code);
> -
> pad_cfg = v4l2_subdev_alloc_pad_config(sd);
> if (pad_cfg == NULL)
> return -ENOMEM;
>
> - format.pad = vin->digital->source_pad;
> + v4l2_fill_mbus_format(&format.format, pix, vin->digital->code);
>
> /* Allow the video device to override field and to scale */
> field = pix->field;
> @@ -217,34 +212,34 @@ static int __rvin_try_format_source(struct rvin_dev
> *vin,
>
> v4l2_fill_pix_format(pix, &format.format);
>
> - source->width = pix->width;
> - source->height = pix->height;
> + crop->top = crop->left = 0;
> + crop->width = pix->width;
> + crop->height = pix->height;
> +
> + /*
> + * If source is ALTERNATE the driver will use the VIN hardware
> + * to INTERLACE it. The crop height then needs to be doubled.
> + */
> + if (pix->field == V4L2_FIELD_ALTERNATE)
> + crop->height *= 2;
> +
> + if (field != V4L2_FIELD_ANY)
> + pix->field = field;
>
> - pix->field = field;
> pix->width = width;
> pix->hei
Re: [PATCH v11 17/32] rcar-vin: move media bus configuration to struct rvin_info
Hi Niklas,
Thank you for the patch.
On Friday, 2 March 2018 03:57:36 EET Niklas Söderlund wrote:
> Bus configuration will once the driver is extended to support Gen3
> contain information not specific to only the directly connected parallel
> subdevice. Move it to struct rvin_dev to show it's not always coupled
> to the parallel subdevice.
The subject line still mentions rvin_info. Are you so emotionally attached to
it that you have trouble fixing that ? ;-)
> Signed-off-by: Niklas Söderlund
> Reviewed-by: Hans Verkuil
> ---
> drivers/media/platform/rcar-vin/rcar-core.c | 18 +-
> drivers/media/platform/rcar-vin/rcar-dma.c | 11 ++-
> drivers/media/platform/rcar-vin/rcar-v4l2.c | 2 +-
> drivers/media/platform/rcar-vin/rcar-vin.h | 9 -
> 4 files changed, 20 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/media/platform/rcar-vin/rcar-core.c
> b/drivers/media/platform/rcar-vin/rcar-core.c index
> cc863e4ec9a4d4b3..449175c3133e42c6 100644
> --- a/drivers/media/platform/rcar-vin/rcar-core.c
> +++ b/drivers/media/platform/rcar-vin/rcar-core.c
> @@ -65,10 +65,10 @@ static int rvin_digital_subdevice_attach(struct rvin_dev
> *vin, vin->digital->sink_pad = ret < 0 ? 0 : ret;
>
> /* Find compatible subdevices mbus format */
> - vin->digital->code = 0;
> + vin->mbus_code = 0;
> code.index = 0;
> code.pad = vin->digital->source_pad;
> - while (!vin->digital->code &&
> + while (!vin->mbus_code &&
> !v4l2_subdev_call(subdev, pad, enum_mbus_code, NULL, &code)) {
> code.index++;
> switch (code.code) {
> @@ -76,16 +76,16 @@ static int rvin_digital_subdevice_attach(struct rvin_dev
> *vin, case MEDIA_BUS_FMT_UYVY8_2X8:
> case MEDIA_BUS_FMT_UYVY10_2X10:
> case MEDIA_BUS_FMT_RGB888_1X24:
> - vin->digital->code = code.code;
> + vin->mbus_code = code.code;
> vin_dbg(vin, "Found media bus format for %s: %d\n",
> - subdev->name, vin->digital->code);
> + subdev->name, vin->mbus_code);
> break;
> default:
> break;
> }
> }
>
> - if (!vin->digital->code) {
> + if (!vin->mbus_code) {
> vin_err(vin, "Unsupported media bus format for %s\n",
> subdev->name);
> return -EINVAL;
> @@ -190,16 +190,16 @@ static int rvin_digital_parse_v4l2(struct device *dev,
> if (vep->base.port || vep->base.id)
> return -ENOTCONN;
>
> - rvge->mbus_cfg.type = vep->bus_type;
> + vin->mbus_cfg.type = vep->bus_type;
>
> - switch (rvge->mbus_cfg.type) {
> + switch (vin->mbus_cfg.type) {
> case V4L2_MBUS_PARALLEL:
> vin_dbg(vin, "Found PARALLEL media bus\n");
> - rvge->mbus_cfg.flags = vep->bus.parallel.flags;
> + vin->mbus_cfg.flags = vep->bus.parallel.flags;
> break;
> case V4L2_MBUS_BT656:
> vin_dbg(vin, "Found BT656 media bus\n");
> - rvge->mbus_cfg.flags = 0;
> + vin->mbus_cfg.flags = 0;
> break;
> default:
> vin_err(vin, "Unknown media bus type\n");
> diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c
> b/drivers/media/platform/rcar-vin/rcar-dma.c index
> c8831e189d362c8b..4ebf76c30a3e9117 100644
> --- a/drivers/media/platform/rcar-vin/rcar-dma.c
> +++ b/drivers/media/platform/rcar-vin/rcar-dma.c
> @@ -633,7 +633,7 @@ static int rvin_setup(struct rvin_dev *vin)
> /*
>* Input interface
>*/
> - switch (vin->digital->code) {
> + switch (vin->mbus_code) {
> case MEDIA_BUS_FMT_YUYV8_1X16:
> /* BT.601/BT.1358 16bit YCbCr422 */
> vnmc |= VNMC_INF_YUV16;
> @@ -641,7 +641,7 @@ static int rvin_setup(struct rvin_dev *vin)
> break;
> case MEDIA_BUS_FMT_UYVY8_2X8:
> /* BT.656 8bit YCbCr422 or BT.601 8bit YCbCr422 */
> - vnmc |= vin->digital->mbus_cfg.type == V4L2_MBUS_BT656 ?
> + vnmc |= vin->mbus_cfg.type == V4L2_MBUS_BT656 ?
> VNMC_INF_YUV8_BT656 : VNMC_INF_YUV8_BT601;
> input_is_yuv = true;
> break;
> @@ -650,7 +650,7 @@ static int rvin_setup(struct rvin_dev *vin)
> break;
> case MEDIA_BUS_FMT_UYVY10_2X10:
> /* BT.656 10bit YCbCr422 or BT.601 10bit YCbCr422 */
> - vnmc |= vin->digital->mbus_cfg.type == V4L2_MBUS_BT656 ?
> + vnmc |= vin->mbus_cfg.type == V4L2_MBUS_BT656 ?
> VNMC_INF_YUV10_BT656 : VNMC_INF_YUV10_BT601;
> input_is_yuv = true;
> break;
> @@ -662,11 +662,11 @@ static int rvin_setup(struct rvin_dev *vin)
> dmr2 = VNDMR2_FTEV | VNDMR2_VLV(1);
>
> /* Hsync Signal Polarity Select */
> - if (!(vi
Re: [PATCH v11 19/32] rcar-vin: add function to manipulate Gen3 chsel value
Hi Niklas,
Thank you for the patch.
On Friday, 2 March 2018 03:57:38 EET Niklas Söderlund wrote:
> On Gen3 the CSI-2 routing is controlled by the VnCSI_IFMD register. One
> feature of this register is that it's only present in the VIN0 and VIN4
> instances. The register in VIN0 controls the routing for VIN0-3 and the
> register in VIN4 controls routing for VIN4-7.
>
> To be able to control routing from a media device this function is need
> to control runtime PM for the subgroup master (VIN0 and VIN4). The
> subgroup master must be switched on before the register is manipulated,
> once the operation is complete it's safe to switch the master off and
> the new routing will still be in effect.
>
> Signed-off-by: Niklas Söderlund
Reviewed-by: Laurent Pinchart
> ---
> drivers/media/platform/rcar-vin/rcar-dma.c | 38 +++
> drivers/media/platform/rcar-vin/rcar-vin.h | 2 ++
> 2 files changed, 40 insertions(+)
By the way it would be useful if you added per-patch changelogs. You can
capture them in the commit message below a --- line.
> diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c
> b/drivers/media/platform/rcar-vin/rcar-dma.c index
> 57bb288b3ca67a60..3fb9c325285c5a5a 100644
> --- a/drivers/media/platform/rcar-vin/rcar-dma.c
> +++ b/drivers/media/platform/rcar-vin/rcar-dma.c
> @@ -16,6 +16,7 @@
>
> #include
> #include
> +#include
>
> #include
>
> @@ -1228,3 +1229,40 @@ int rvin_dma_register(struct rvin_dev *vin, int irq)
>
> return ret;
> }
> +
> +/*
> + * Gen3 CHSEL manipulation
> + */
> +
> +/*
> + * There is no need to have locking around changing the routing
> + * as it's only possible to do so when no VIN in the group is
> + * streaming so nothing can race with the VNMC register.
> + */
> +int rvin_set_channel_routing(struct rvin_dev *vin, u8 chsel)
> +{
> + u32 ifmd, vnmc;
> + int ret;
> +
> + ret = pm_runtime_get_sync(vin->dev);
> + if (ret < 0)
> + return ret;
> +
> + /* Make register writes take effect immediately. */
> + vnmc = rvin_read(vin, VNMC_REG);
> + rvin_write(vin, vnmc & ~VNMC_VUP, VNMC_REG);
> +
> + ifmd = VNCSI_IFMD_DES2 | VNCSI_IFMD_DES1 | VNCSI_IFMD_DES0 |
> + VNCSI_IFMD_CSI_CHSEL(chsel);
> +
> + rvin_write(vin, ifmd, VNCSI_IFMD_REG);
> +
> + vin_dbg(vin, "Set IFMD 0x%x\n", ifmd);
> +
> + /* Restore VNMC. */
> + rvin_write(vin, vnmc, VNMC_REG);
> +
> + pm_runtime_put(vin->dev);
> +
> + return ret;
> +}
> diff --git a/drivers/media/platform/rcar-vin/rcar-vin.h
> b/drivers/media/platform/rcar-vin/rcar-vin.h index
> b3802651eaa78ea9..666308946eb4994d 100644
> --- a/drivers/media/platform/rcar-vin/rcar-vin.h
> +++ b/drivers/media/platform/rcar-vin/rcar-vin.h
> @@ -165,4 +165,6 @@ const struct rvin_video_format
> *rvin_format_from_pixel(u32 pixelformat); /* Cropping, composing and
> scaling */
> void rvin_crop_scale_comp(struct rvin_dev *vin);
>
> +int rvin_set_channel_routing(struct rvin_dev *vin, u8 chsel);
> +
> #endif
--
Regards,
Laurent Pinchart
Re: [PATCH v9 19/28] rcar-vin: use different v4l2 operations in media controller mode
Hi Niklas,
On Friday, 19 January 2018 02:46:03 EET Niklas Söderlund wrote:
> Hi Laurent,
>
> Thanks for your comments.
>
> Apart from the issue with the input API Hans pointed which indicates I
> need to keep that around until it's fixed in the framework I agree with
> all your comments but one.
>
>
>
> On 2017-12-08 12:14:05 +0200, Laurent Pinchart wrote:
> >> @@ -628,7 +628,8 @@ static int rvin_setup(struct rvin_dev *vin)
> >>/* Default to TB */
> >>vnmc = VNMC_IM_FULL;
> >>/* Use BT if video standard can be read and is 60 Hz format */
> >> - if (!v4l2_subdev_call(vin_to_source(vin), video, g_std, &std)) {
> >> + if (!vin->info->use_mc &&
> >> + !v4l2_subdev_call(vin_to_source(vin), video, g_std, &std)) {
> >>if (std & V4L2_STD_525_60)
> >>vnmc = VNMC_IM_FULL | VNMC_FOC;
> >>}
> >
> > I think the subdev should be queried in rcar-v4l2.c and the information
> > cached in the rvin_dev structure instead of queried here. Interactions
> > with the subdev should be minimized in rvin-dma.c. You can fix this in a
> > separate patch if you prefer.
>
> This can't be a cached value it needs to be read at stream on time. The
> input source could have changed format, e.g. the user may have
> disconnected a NTSC source and plugged in a PAL.
Please note that in that case the source subdev will not have changed its
format yet, it only does so when the s_std operation is called.
> This is a shame as I otherwise agree with you that interactions with the
> subdevice should be kept at a minimum.
>
>
--
Regards,
Laurent Pinchart
Re: [PATCH v11 21/32] rcar-vin: use different v4l2 operations in media controller mode
Hi Niklas,
Thank you for the patch.
On Friday, 2 March 2018 03:57:40 EET Niklas Söderlund wrote:
> When the driver runs in media controller mode it should not directly
> control the subdevice instead userspace will be responsible for
> configuring the pipeline. To be able to run in this mode a different set
> of v4l2 operations needs to be used.
>
> Add a new set of v4l2 operations to support operation without directly
> interacting with the source subdevice.
>
> Signed-off-by: Niklas Söderlund
> Reviewed-by: Hans Verkuil
> ---
> drivers/media/platform/rcar-vin/rcar-dma.c | 3 +-
> drivers/media/platform/rcar-vin/rcar-v4l2.c | 161 -
> 2 files changed, 160 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c
> b/drivers/media/platform/rcar-vin/rcar-dma.c index
> 3fb9c325285c5a5a..27d0c098f1da40f9 100644
> --- a/drivers/media/platform/rcar-vin/rcar-dma.c
> +++ b/drivers/media/platform/rcar-vin/rcar-dma.c
> @@ -628,7 +628,8 @@ static int rvin_setup(struct rvin_dev *vin)
> /* Default to TB */
> vnmc = VNMC_IM_FULL;
> /* Use BT if video standard can be read and is 60 Hz format */
> - if (!v4l2_subdev_call(vin_to_source(vin), video, g_std, &std)) {
> + if (!vin->info->use_mc &&
> + !v4l2_subdev_call(vin_to_source(vin), video, g_std, &std)) {
To avoid spreading the discussion across several mail threads, I'll repeat my
comment made today on v9:
> > I think the subdev should be queried in rcar-v4l2.c and the information
> > cached in the rvin_dev structure instead of queried here. Interactions
> > with the subdev should be minimized in rvin-dma.c. You can fix this in a
> > separate patch if you prefer.
>
> This can't be a cached value it needs to be read at stream on time. The
> input source could have changed format, e.g. the user may have
> disconnected a NTSC source and plugged in a PAL.
Please note that in that case the source subdev will not have changed its
format yet, it only does so when the s_std operation is called.
> This is a shame as I otherwise agree with you that interactions with the
> subdevice should be kept at a minimum.
> if (std & V4L2_STD_525_60)
> vnmc = VNMC_IM_FULL | VNMC_FOC;
> }
> diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c
> b/drivers/media/platform/rcar-vin/rcar-v4l2.c index
> 20be21cb1cf521e5..8d92710efffa7276 100644
> --- a/drivers/media/platform/rcar-vin/rcar-v4l2.c
> +++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c
> @@ -18,12 +18,16 @@
>
> #include
> #include
> +#include
> #include
>
> #include "rcar-vin.h"
>
> #define RVIN_DEFAULT_FORMAT V4L2_PIX_FMT_YUYV
> +#define RVIN_DEFAULT_WIDTH 800
> +#define RVIN_DEFAULT_HEIGHT 600
> #define RVIN_DEFAULT_FIELD V4L2_FIELD_NONE
> +#define RVIN_DEFAULT_COLORSPACE V4L2_COLORSPACE_SRGB
>
> /*
> * Format Conversions
> @@ -667,6 +671,74 @@ static const struct v4l2_ioctl_ops rvin_ioctl_ops = {
> .vidioc_unsubscribe_event = v4l2_event_unsubscribe,
> };
>
> +/*
> + * V4L2 Media Controller
> + */
> +
> +static int rvin_mc_try_fmt_vid_cap(struct file *file, void *priv,
> +struct v4l2_format *f)
> +{
> + struct rvin_dev *vin = video_drvdata(file);
> +
> + return rvin_format_align(vin, &f->fmt.pix);
> +}
> +
> +static int rvin_mc_s_fmt_vid_cap(struct file *file, void *priv,
> + struct v4l2_format *f)
> +{
> + struct rvin_dev *vin = video_drvdata(file);
> + int ret;
> +
> + if (vb2_is_busy(&vin->queue))
> + return -EBUSY;
> +
> + ret = rvin_format_align(vin, &f->fmt.pix);
> + if (ret)
> + return ret;
> +
> + vin->format = f->fmt.pix;
> +
> + return 0;
> +}
> +
> +static int rvin_mc_enum_input(struct file *file, void *priv,
> + struct v4l2_input *i)
> +{
> + if (i->index != 0)
> + return -EINVAL;
> +
> + i->type = V4L2_INPUT_TYPE_CAMERA;
> + strlcpy(i->name, "Camera", sizeof(i->name));
> +
> + return 0;
> +}
> +
> +static const struct v4l2_ioctl_ops rvin_mc_ioctl_ops = {
> + .vidioc_querycap= rvin_querycap,
> + .vidioc_try_fmt_vid_cap = rvin_mc_try_fmt_vid_cap,
> + .vidioc_g_fmt_vid_cap = rvin_g_fmt_vid_cap,
> + .vidioc_s_fmt_vid_cap = rvin_mc_s_fmt_vid_cap,
> + .vidioc_enum_fmt_vid_cap= rvin_enum_fmt_vid_cap,
> +
> + .vidioc_enum_input = rvin_mc_enum_input,
> + .vidioc_g_input = rvin_g_input,
> + .vidioc_s_input = rvin_s_input,
> +
> + .vidioc_reqbufs = vb2_ioctl_reqbufs,
> + .vidioc_create_bufs =
Re: [PATCH v11 26/32] rcar-vin: add chsel information to rvin_info
Hi Niklas,
Thank you for the patch.
On Friday, 2 March 2018 03:57:45 EET Niklas Söderlund wrote:
> Each Gen3 SoC has a limited set of predefined routing possibilities for
> which CSI-2 device and channel can be routed to which VIN instance.
> Prepare to store this information in the struct rvin_info.
>
> Signed-off-by: Niklas Söderlund
> ---
> drivers/media/platform/rcar-vin/rcar-vin.h | 42 +++
> 1 file changed, 42 insertions(+)
>
> diff --git a/drivers/media/platform/rcar-vin/rcar-vin.h
> b/drivers/media/platform/rcar-vin/rcar-vin.h index
> 07cde9e1ab01ca51..6150a883e17f8479 100644
> --- a/drivers/media/platform/rcar-vin/rcar-vin.h
> +++ b/drivers/media/platform/rcar-vin/rcar-vin.h
> @@ -43,6 +43,14 @@ enum model_id {
> RCAR_GEN3,
> };
>
> +enum rvin_csi_id {
> + RVIN_CSI20,
> + RVIN_CSI21,
> + RVIN_CSI40,
> + RVIN_CSI41,
> + RVIN_CSI_MAX,
> +};
> +
> /**
> * STOPPED - No operation in progress
> * RUNNING - Operation in progress have buffers
> @@ -81,12 +89,45 @@ struct rvin_graph_entity {
> unsigned int sink_pad;
> };
>
> +/**
> + * struct rvin_group_route - describes a route from a channel of a
> + * CSI-2 receiver to a VIN
> + *
> + * @vin: VIN ID.
> + * @csi: CSI-2 receiver ID.
> + * @chan:Output channel of the CSI-2 receiver.
Do you think channel instead of chan would be too long ?
> + * @mask:Bitmask of the different CHSEL register values that
> + * allows for a route from @csi + @chan to @vin.
s/allows/allow/
> + *
> + * .. note::
> + * Each R-Car CSI-2 receiver has four output channels facing the VIN
> + * devices, each channel can carry one CSI-2 Virtual Channel (VC).
> + * There are no correlation between channel number and CSI-2 VC. It's
s/are/is/
> + * up to the CSI-2 receiver driver to configure which VC is output
> + * on which channel, the VIN devices only care about output channels.
> + *
> + * There are in some cases multiple CHSEL register settings which would
> + * allow for the same route from @csi + @chan to @vin. For example
> + * on R-Car H3 both the CHSEL values 0 and 3 allows for a route from
s/allows/allow/
> + * CSI40/VC0 to VIN0. All possible CHSEL values for a route need to be
> + * recorded as a bitmask in @mask, in this example bit 0 and 3 should
> + * be set.
> + */
> +struct rvin_group_route {
> + unsigned int vin;
> + enum rvin_csi_id csi;
> + unsigned char chan;
> + unsigned int mask;
> +};
Repeating my comments on v10,
You can make chan an unsigned int, the compiler will pad the field anyway.
I think it would be clearer to order the fields in "from -> to: configuration"
order (csi, channel, vin, mask).
> /**
> * struct rvin_info - Information about the particular VIN implementation
> * @model: VIN model
> * @use_mc: use media controller instead of controlling subdevice
> * @max_width: max input width the VIN supports
> * @max_height: max input height the VIN supports
> + * @routes: list of possible routes from the CSI-2 recivers to
> + * all VINs. The list mush be NULL terminated.
> */
> struct rvin_info {
> enum model_id model;
> @@ -94,6 +135,7 @@ struct rvin_info {
>
> unsigned int max_width;
> unsigned int max_height;
> + const struct rvin_group_route *routes;
> };
>
> /**
--
Regards,
Laurent Pinchart
Re: [PATCH v11 28/32] rcar-vin: add link notify for Gen3
Hi Niklas,
Thank you for the patch.
On Friday, 2 March 2018 03:57:47 EET Niklas Söderlund wrote:
> Add the ability to process media device link change request. Link
s/request/requests/
> enabling is a bit complicated on Gen3, whether or not it's possible to
> enable a link depends on what other links already are enabled. On Gen3
> the 8 VINs are split into two subgroup's (VIN0-3 and VIN4-7) and from a
> routing perspective these two groups are independent of each other.
> Each subgroup's routing is controlled by the subgroup VIN master
> instance (VIN0 and VIN4).
>
> There are a limited number of possible route setups available for each
> subgroup and the configuration of each setup is dictated by the
> hardware. On H3 for example there are 6 possible route setups for each
> subgroup to choose from.
>
> This leads to the media device link notification code being rather large
> since it will find the best routing configuration to try and accommodate
> as many links as possible. When it's not possible to enable a new link
> due to hardware constrains the link_notifier callback will return
> -EMLINK.
>
> Signed-off-by: Niklas Söderlund
> ---
> drivers/media/platform/rcar-vin/rcar-core.c | 153 +
> 1 file changed, 153 insertions(+)
>
> diff --git a/drivers/media/platform/rcar-vin/rcar-core.c
> b/drivers/media/platform/rcar-vin/rcar-core.c index
> 3ddd7d8fecd52909..c015f3c284439285 100644
> --- a/drivers/media/platform/rcar-vin/rcar-core.c
> +++ b/drivers/media/platform/rcar-vin/rcar-core.c
> @@ -24,6 +24,7 @@
>
> #include
> #include
> +#include
>
> #include "rcar-vin.h"
>
> @@ -44,6 +45,157 @@
> */
> #define rvin_group_id_to_master(vin) ((vin) < 4 ? 0 : 4)
>
> +/*
> + * Media Controller link notification
> + */
> +
> +/* group lock should be held when calling this function. */
> +static int rvin_group_entity_to_csi_id(struct rvin_group *group,
> +struct media_entity *entity)
> +{
> + struct v4l2_subdev *sd;
> + unsigned int i;
> +
> + sd = media_entity_to_v4l2_subdev(entity);
> +
> + for (i = 0; i < RVIN_CSI_MAX; i++)
> + if (group->csi[i].subdev == sd)
> + return i;
> +
> + return -ENODEV;
> +}
> +
> +static unsigned int rvin_group_get_mask(struct rvin_dev *vin,
> + enum rvin_csi_id csi_id,
> + unsigned char chan)
> +{
> + const struct rvin_group_route *route;
> + unsigned int mask = 0;
> +
> + for (route = vin->info->routes; route->mask; route++) {
> + if (route->vin == vin->id &&
> + route->csi == csi_id &&
> + route->chan == chan) {
> + vin_dbg(vin, "Adding route: vin: %d csi: %d chan: %d\n",
> + route->vin, route->csi, route->chan);
> + mask |= route->mask;
> + }
> + }
> +
> + return mask;
> +}
> +
> +/*
> + * Link setup for the links between a VIN and a CSI-2 receiver is a bit
> + * complex. The reason for this is that the register controlling routing
> + * are not present in each VIN instance. There are special VINs which
s/are/is/
> + * control routing for itself and other VINs. There are not many
s/itself/themselves/
> + * different possible links combinations that can be enabled at the same
> + * time, therefor all already enabled links which are controlled by a
> + * master VIN needs to be taken into account when making the decision
s/needs/need/
> + * if a new link can be enabled or not.
> + *
> + * 1. Find out which VIN the link the user tries to enable is connected to.
> + * 2. Lookup which master VIN controls the links for this VIN.
> + * 3. Start with a bitmask with all bits set.
> + * 4. For each previously enabled link from the master VIN bitwise AND its
> + *route mask (see documentation for mask in struct rvin_group_route)
> + *with the bitmask.
> + * 5. Bitwise AND the mask for the link the user tries to enable to the
> bitmask.
> + * 6. If the bitmask is not empty at this point the new link can be enabled
> + *while keeping all previous links enabled. Update the CHSEL value of
> the
> + *master VIN and inform the user that the link could be enabled.
> + *
> + * Please note that no link can be enabled if any VIN in the group is
> + * currently streaming.
> + */
> +static int rvin_group_link_notify(struct media_link *link, u32 flags,
> + unsigned int notification)
> +{
> + struct rvin_group *group = container_of(link->graph_obj.mdev,
> + struct rvin_group, mdev);
> + unsigned int master_id, chan, mask_new, i;
> + unsigned int mask = ~0;
> + struct media_entity *entity;
> + struct video_device *vdev;
> + struct media_pad *csi_pad;
> + struct rvin_dev *vi
Re: [RESEND PATCH v5 0/6] IR support for A83T
On 13.02.2018 13:29, Philipp Rossak wrote: This patch series adds support for the sunxi A83T ir module and enhances the sunxi-ir driver. Right now the base clock frequency for the ir driver is a hard coded define and is set to 8 MHz. This works for the most common ir receivers. On the Sinovoip Bananapi M3 the ir receiver needs, a 3 MHz base clock frequency to work without problems with this driver. This patch series adds support for an optinal property that makes it able to override the default base clock frequency and enables the ir interface on the a83t and the Bananapi M3. changes since v4: * rename cir pin from cir_pins to r_cir_pin * drop unit-adress from r_cir_pin * add a83t compatible to the cir node * move muxing options to dtsi * rename cir label and reorder it in the bananpim3.dts file changes since v3: * collecting all acks & reviewd by * fixed typos changes since v2: * reorder cir pin (alphabetical) * fix typo in documentation changes since v1: * fix typos, reword Documentation * initialize 'b_clk_freq' to 'SUNXI_IR_BASE_CLK' & remove if statement * change dev_info() to dev_dbg() * change naming to cir* in dts/dtsi * Added acked Ackedi-by to related patch * use whole memory block instead of registers needed + fix for h3/h5 changes since rfc: * The property is now optinal. If the property is not available in the dtb the driver uses the default base clock frequency. * the driver prints out the the selected base clock frequency. * changed devicetree property from base-clk-frequency to clock-frequency Regards, Philipp Philipp Rossak (6): media: rc: update sunxi-ir driver to get base clock frequency from devicetree media: dt: bindings: Update binding documentation for sunxi IR controller arm: dts: sun8i: a83t: Add the cir pin for the A83T arm: dts: sun8i: a83t: Add support for the cir interface arm: dts: sun8i: a83t: bananapi-m3: Enable IR controller arm: dts: sun8i: h3-h5: ir register size should be the whole memory block Documentation/devicetree/bindings/media/sunxi-ir.txt | 3 +++ arch/arm/boot/dts/sun8i-a83t-bananapi-m3.dts | 5 + arch/arm/boot/dts/sun8i-a83t.dtsi| 18 ++ arch/arm/boot/dts/sunxi-h3-h5.dtsi | 2 +- drivers/media/rc/sunxi-cir.c | 19 +++ 5 files changed, 38 insertions(+), 9 deletions(-) I talked yesterday with Maxime about this patch series. And he told me if the first to patches got merged, he will apply the dts patches to the sunxi tree. Sean, can you merge the first two patches through the rc-core? Thanks, Philipp
Re: [RESEND PATCH v5 0/6] IR support for A83T
On Fri, Mar 02, 2018 at 01:11:34PM +0100, Philipp Rossak wrote: > On 13.02.2018 13:29, Philipp Rossak wrote: > > This patch series adds support for the sunxi A83T ir module and enhances > > the sunxi-ir driver. Right now the base clock frequency for the ir driver > > is a hard coded define and is set to 8 MHz. > > This works for the most common ir receivers. On the Sinovoip Bananapi M3 > > the ir receiver needs, a 3 MHz base clock frequency to work without > > problems with this driver. > > > > This patch series adds support for an optinal property that makes it able > > to override the default base clock frequency and enables the ir interface > > on the a83t and the Bananapi M3. > > > > changes since v4: > > * rename cir pin from cir_pins to r_cir_pin > > * drop unit-adress from r_cir_pin > > * add a83t compatible to the cir node > > * move muxing options to dtsi > > * rename cir label and reorder it in the bananpim3.dts file > > > > changes since v3: > > * collecting all acks & reviewd by > > * fixed typos > > > > changes since v2: > > * reorder cir pin (alphabetical) > > * fix typo in documentation > > > > changes since v1: > > * fix typos, reword Documentation > > * initialize 'b_clk_freq' to 'SUNXI_IR_BASE_CLK' & remove if statement > > * change dev_info() to dev_dbg() > > * change naming to cir* in dts/dtsi > > * Added acked Ackedi-by to related patch > > * use whole memory block instead of registers needed + fix for h3/h5 > > > > changes since rfc: > > * The property is now optinal. If the property is not available in > >the dtb the driver uses the default base clock frequency. > > * the driver prints out the the selected base clock frequency. > > * changed devicetree property from base-clk-frequency to clock-frequency > > > > Regards, > > Philipp > > > > Philipp Rossak (6): > >media: rc: update sunxi-ir driver to get base clock frequency from > > devicetree > >media: dt: bindings: Update binding documentation for sunxi IR > > controller > >arm: dts: sun8i: a83t: Add the cir pin for the A83T > >arm: dts: sun8i: a83t: Add support for the cir interface > >arm: dts: sun8i: a83t: bananapi-m3: Enable IR controller > >arm: dts: sun8i: h3-h5: ir register size should be the whole memory > > block > > > > Documentation/devicetree/bindings/media/sunxi-ir.txt | 3 +++ > > arch/arm/boot/dts/sun8i-a83t-bananapi-m3.dts | 5 + > > arch/arm/boot/dts/sun8i-a83t.dtsi| 18 > > ++ > > arch/arm/boot/dts/sunxi-h3-h5.dtsi | 2 +- > > drivers/media/rc/sunxi-cir.c | 19 > > +++ > > 5 files changed, 38 insertions(+), 9 deletions(-) > > > > I talked yesterday with Maxime about this patch series. And he told me if > the first to patches got merged, he will apply the dts patches to the sunxi > tree. > > Sean, can you merge the first two patches through the rc-core? Sure, no problem. Thanks Sean
Re: [bug report] media: i2c: Copy tw9910 soc_camera sensor driver
Hi Dan,
On Thu, Mar 01, 2018 at 12:59:54PM +0300, Dan Carpenter wrote:
> [ I know you're just copying files, but you might have a fix for these
> since you're looking at the code. - dan ]
According to the static analyzer I should simply substitute all those
expressions with 0s. I would instead keep them for sake of readability
and accordance with register description in the video decoder manual.
Thanks
j
>
> Hello Jacopo Mondi,
>
> The patch e0d76c3842ee: "media: i2c: Copy tw9910 soc_camera sensor
> driver" from Feb 21, 2018, leads to the following static checker
> warning:
>
> drivers/media/i2c/tw9910.c:395 tw9910_set_hsync()
> warn: odd binop '0x260 & 0x7'
>
> drivers/media/i2c/tw9910.c:396 tw9910_set_hsync()
> warn: odd binop '0x300 & 0x7'
>
> drivers/media/i2c/tw9910.c:538 tw9910_s_std()
> warn: odd binop '0x0 & 0xc'
>
> drivers/media/i2c/tw9910.c
>374 static int tw9910_set_hsync(struct i2c_client *client)
>375 {
>376 struct tw9910_priv *priv = to_tw9910(client);
>377 int ret;
>378
>379 /* bit 10 - 3 */
>380 ret = i2c_smbus_write_byte_data(client, HSBEGIN,
>381 (HSYNC_START & 0x07F8) >> 3);
>382 if (ret < 0)
>383 return ret;
>384
>385 /* bit 10 - 3 */
>386 ret = i2c_smbus_write_byte_data(client, HSEND,
>387 (HSYNC_END & 0x07F8) >> 3);
>388 if (ret < 0)
>389 return ret;
>390
>391 /* So far only revisions 0 and 1 have been seen */
>392 /* bit 2 - 0 */
>393 if (priv->revision == 1)
>394 ret = tw9910_mask_set(client, HSLOWCTL, 0x77,
>395(HSYNC_START & 0x0007) << 4 |
>
>396(HSYNC_END & 0x0007));
>
> These always mask to zero.
>
>397
>398 return ret;
>399 }
>
> [ snip ]
>
>511 static int tw9910_s_std(struct v4l2_subdev *sd, v4l2_std_id norm)
>512 {
>513 struct i2c_client *client = v4l2_get_subdevdata(sd);
>514 struct tw9910_priv *priv = to_tw9910(client);
>515 const unsigned int hact = 720;
>516 const unsigned int hdelay = 15;
>^^^
>517 unsigned int vact;
>518 unsigned int vdelay;
>519 int ret;
>520
>521 if (!(norm & (V4L2_STD_NTSC | V4L2_STD_PAL)))
>522 return -EINVAL;
>523
>524 priv->norm = norm;
>525 if (norm & V4L2_STD_525_60) {
>526 vact = 240;
>527 vdelay = 18;
>528 ret = tw9910_mask_set(client, VVBI, 0x10, 0x10);
>529 } else {
>530 vact = 288;
>531 vdelay = 24;
>532 ret = tw9910_mask_set(client, VVBI, 0x10, 0x00);
>533 }
>534 if (!ret)
>535 ret = i2c_smbus_write_byte_data(client, CROP_HI,
>536 ((vdelay >> 2) &
> 0xc0) |
>537 ((vact >> 4) & 0x30) |
>538 ((hdelay >> 6) & 0x0c) |
> ^^^
> 15 >> 6 is zero.
>
>539 ((hact >> 8) & 0x03));
>540 if (!ret)
>541 ret = i2c_smbus_write_byte_data(client, VDELAY_LO,
>542 vdelay & 0xff);
>543 if (!ret)
>544 ret = i2c_smbus_write_byte_data(client, VACTIVE_LO,
>545 vact & 0xff);
>546
>547 return ret;
>548 }
>
> regards,
> dan carpenter
[PATCH 05/12] media: ov5640: Change horizontal and vertical resolutions name
The current width and height parameters in the struct ov5640_mode_info are
actually the active horizontal and vertical resolutions.
Since we're going to add a few other parameters, let's pick a better, more
precise name for these values.
Signed-off-by: Maxime Ripard
---
drivers/media/i2c/ov5640.c | 22 +++---
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index 0415484e32fb..8c04f2880715 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -166,8 +166,8 @@ struct reg_value {
struct ov5640_mode_info {
enum ov5640_mode_id id;
enum ov5640_downsize_mode dn_mode;
- u32 width;
- u32 height;
+ u32 hact;
+ u32 vact;
const struct reg_value *reg_data;
u32 reg_data_size;
};
@@ -1388,10 +1388,10 @@ ov5640_find_mode(struct ov5640_dev *sensor, enum
ov5640_frame_rate fr,
if (!mode->reg_data)
continue;
- if ((nearest && mode->width <= width &&
-mode->height <= height) ||
- (!nearest && mode->width == width &&
-mode->height == height))
+ if ((nearest && mode->hact <= width &&
+mode->vact <= height) ||
+ (!nearest && mode->hact == width &&
+mode->vact == height))
break;
}
@@ -1871,8 +1871,8 @@ static int ov5640_try_fmt_internal(struct v4l2_subdev *sd,
mode = ov5640_find_mode(sensor, fr, fmt->width, fmt->height, true);
if (!mode)
return -EINVAL;
- fmt->width = mode->width;
- fmt->height = mode->height;
+ fmt->width = mode->hact;
+ fmt->height = mode->vact;
if (new_mode)
*new_mode = mode;
@@ -2304,9 +2304,9 @@ static int ov5640_enum_frame_size(struct v4l2_subdev *sd,
return -EINVAL;
fse->min_width = fse->max_width =
- ov5640_mode_data[0][fse->index].width;
+ ov5640_mode_data[0][fse->index].hact;
fse->min_height = fse->max_height =
- ov5640_mode_data[0][fse->index].height;
+ ov5640_mode_data[0][fse->index].vact;
return 0;
}
@@ -2369,7 +2369,7 @@ static int ov5640_s_frame_interval(struct v4l2_subdev *sd,
mode = sensor->current_mode;
frame_rate = ov5640_try_frame_interval(sensor, &fi->interval,
- mode->width, mode->height);
+ mode->hact, mode->vact);
if (frame_rate < 0)
frame_rate = OV5640_15_FPS;
--
2.14.3
[PATCH 00/12] media: ov5640: Misc cleanup and improvements
Hi, Here is a "small" series that mostly cleans up the ov5640 driver code, slowly getting rid of the big data array for more understandable code (hopefully). The biggest addition would be the clock rate computation at runtime, instead of relying on those arrays to setup the clock tree properly. As a side effect, it fixes the framerate that was off by around 10% on the smaller resolutions, and we now support 60fps. This also introduces a bunch of new features. Let me know what you think, Maxime Maxime Ripard (10): media: ov5640: Don't force the auto exposure state at start time media: ov5640: Init properly the SCLK dividers media: ov5640: Change horizontal and vertical resolutions name media: ov5640: Add horizontal and vertical totals media: ov5640: Program the visible resolution media: ov5640: Adjust the clock based on the expected rate media: ov5640: Compute the clock rate at runtime media: ov5640: Enhance FPS handling media: ov5640: Add 60 fps support media: ov5640: Remove duplicate auto-exposure setup Mylène Josserand (2): media: ov5640: Add auto-focus feature media: ov5640: Add light frequency control drivers/media/i2c/ov5640.c | 777 ++--- 1 file changed, 452 insertions(+), 325 deletions(-) -- 2.14.3
[PATCH 04/12] media: ov5640: Init properly the SCLK dividers
The SCLK and SCLK2X dividers are fixed in stone in the initialization
array. Let's make explicit what we're doing and move that away from the
huge array to the initialization code.
Signed-off-by: Maxime Ripard
---
drivers/media/i2c/ov5640.c | 11 ++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index 0d8f979416cc..0415484e32fb 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -90,6 +90,9 @@
#define OV5640_REG_SDE_CTRL5 0x5585
#define OV5640_REG_AVG_READOUT 0x56a1
+#define OV5640_SCLK2X_ROOT_DIVIDER_DEFAULT 1
+#define OV5640_SCLK_ROOT_DIVIDER_DEFAULT 2
+
enum ov5640_mode_id {
OV5640_MODE_QCIF_176_144 = 0,
OV5640_MODE_QVGA_320_240,
@@ -249,7 +252,7 @@ static const struct reg_value
ov5640_init_setting_30fps_VGA[] = {
{0x3103, 0x11, 0, 0}, {0x3008, 0x82, 0, 5}, {0x3008, 0x42, 0, 0},
{0x3103, 0x03, 0, 0}, {0x3017, 0x00, 0, 0}, {0x3018, 0x00, 0, 0},
{0x3034, 0x18, 0, 0}, {0x3035, 0x14, 0, 0}, {0x3036, 0x38, 0, 0},
- {0x3037, 0x13, 0, 0}, {0x3108, 0x01, 0, 0}, {0x3630, 0x36, 0, 0},
+ {0x3037, 0x13, 0, 0}, {0x3630, 0x36, 0, 0},
{0x3631, 0x0e, 0, 0}, {0x3632, 0xe2, 0, 0}, {0x3633, 0x12, 0, 0},
{0x3621, 0xe0, 0, 0}, {0x3704, 0xa0, 0, 0}, {0x3703, 0x5a, 0, 0},
{0x3715, 0x78, 0, 0}, {0x3717, 0x01, 0, 0}, {0x370b, 0x60, 0, 0},
@@ -1649,6 +1652,12 @@ static int ov5640_restore_mode(struct ov5640_dev *sensor)
if (ret < 0)
return ret;
+ ret = ov5640_mod_reg(sensor, OV5640_REG_SYS_ROOT_DIVIDER, 0x3f,
+(ilog2(OV5640_SCLK2X_ROOT_DIVIDER_DEFAULT) << 2) |
+ilog2(OV5640_SCLK_ROOT_DIVIDER_DEFAULT));
+ if (ret)
+ return ret;
+
/* now restore the last capture mode */
return ov5640_set_mode(sensor, &ov5640_mode_init_data);
}
--
2.14.3
[PATCH 03/12] media: ov5640: Don't force the auto exposure state at start time
The sensor needs to have the auto exposure stopped while changing mode. However, when the new mode is set, the driver will force the auto exposure on, disregarding whether the control has been changed or not. Bypass the controls code entirely to do that, and only use the control value cached when restoring the auto exposure mode. Signed-off-by: Maxime Ripard --- drivers/media/i2c/ov5640.c | 7 +-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c index e6a23eb55c1d..0d8f979416cc 100644 --- a/drivers/media/i2c/ov5640.c +++ b/drivers/media/i2c/ov5640.c @@ -1579,7 +1579,9 @@ static int ov5640_set_mode_direct(struct ov5640_dev *sensor, ret = __v4l2_ctrl_s_ctrl(sensor->ctrls.auto_gain, 1); if (ret) return ret; - return __v4l2_ctrl_s_ctrl(sensor->ctrls.auto_exp, V4L2_EXPOSURE_AUTO); + + return __v4l2_ctrl_s_ctrl(sensor->ctrls.auto_exp, + sensor->ctrls.auto_exp->val); } static int ov5640_set_mode(struct ov5640_dev *sensor, @@ -1596,7 +1598,8 @@ static int ov5640_set_mode(struct ov5640_dev *sensor, ret = __v4l2_ctrl_s_ctrl(sensor->ctrls.auto_gain, 0); if (ret) return ret; - ret = __v4l2_ctrl_s_ctrl(sensor->ctrls.auto_exp, V4L2_EXPOSURE_MANUAL); + + ret = ov5640_set_exposure(sensor, V4L2_EXPOSURE_MANUAL); if (ret) return ret; -- 2.14.3
[PATCH 07/12] media: ov5640: Program the visible resolution
The active frame size is set in the initialization arrays, but the value
itself is also available in the struct ov5640_mode_info.
Let's move these values out of the big bytes arrays, and program it with
the value of the mode that we are given.
Signed-off-by: Maxime Ripard
---
drivers/media/i2c/ov5640.c | 58 +++---
1 file changed, 14 insertions(+), 44 deletions(-)
diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index 443b167bcd20..0eeb1667bbe7 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -58,6 +58,8 @@
#define OV5640_REG_AEC_PK_MANUAL 0x3503
#define OV5640_REG_AEC_PK_REAL_GAIN0x350a
#define OV5640_REG_AEC_PK_VTS 0x350c
+#define OV5640_REG_TIMING_DVPHO0x3808
+#define OV5640_REG_TIMING_DVPVO0x380a
#define OV5640_REG_TIMING_HTS 0x380c
#define OV5640_REG_TIMING_VTS 0x380e
#define OV5640_REG_TIMING_TC_REG21 0x3821
@@ -271,8 +273,6 @@ static const struct reg_value
ov5640_init_setting_30fps_VGA[] = {
{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
{0x3802, 0x00, 0, 0}, {0x3803, 0x04, 0, 0}, {0x3804, 0x0a, 0, 0},
{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9b, 0, 0},
- {0x3808, 0x02, 0, 0}, {0x3809, 0x80, 0, 0}, {0x380a, 0x01, 0, 0},
- {0x380b, 0xe0, 0, 0},
{0x3810, 0x00, 0, 0},
{0x3811, 0x10, 0, 0}, {0x3812, 0x00, 0, 0}, {0x3813, 0x06, 0, 0},
{0x3618, 0x00, 0, 0}, {0x3612, 0x29, 0, 0}, {0x3708, 0x64, 0, 0},
@@ -346,8 +346,6 @@ static const struct reg_value
ov5640_setting_30fps_VGA_640_480[] = {
{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
{0x3802, 0x00, 0, 0}, {0x3803, 0x04, 0, 0}, {0x3804, 0x0a, 0, 0},
{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9b, 0, 0},
- {0x3808, 0x02, 0, 0}, {0x3809, 0x80, 0, 0}, {0x380a, 0x01, 0, 0},
- {0x380b, 0xe0, 0, 0},
{0x3810, 0x00, 0, 0},
{0x3811, 0x10, 0, 0}, {0x3812, 0x00, 0, 0}, {0x3813, 0x06, 0, 0},
{0x3618, 0x00, 0, 0}, {0x3612, 0x29, 0, 0}, {0x3708, 0x64, 0, 0},
@@ -367,8 +365,6 @@ static const struct reg_value
ov5640_setting_15fps_VGA_640_480[] = {
{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
{0x3802, 0x00, 0, 0}, {0x3803, 0x04, 0, 0}, {0x3804, 0x0a, 0, 0},
{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9b, 0, 0},
- {0x3808, 0x02, 0, 0}, {0x3809, 0x80, 0, 0}, {0x380a, 0x01, 0, 0},
- {0x380b, 0xe0, 0, 0},
{0x3810, 0x00, 0, 0},
{0x3811, 0x10, 0, 0}, {0x3812, 0x00, 0, 0}, {0x3813, 0x06, 0, 0},
{0x3618, 0x00, 0, 0}, {0x3612, 0x29, 0, 0}, {0x3708, 0x64, 0, 0},
@@ -389,8 +385,6 @@ static const struct reg_value
ov5640_setting_30fps_XGA_1024_768[] = {
{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
{0x3802, 0x00, 0, 0}, {0x3803, 0x04, 0, 0}, {0x3804, 0x0a, 0, 0},
{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9b, 0, 0},
- {0x3808, 0x02, 0, 0}, {0x3809, 0x80, 0, 0}, {0x380a, 0x01, 0, 0},
- {0x380b, 0xe0, 0, 0},
{0x3810, 0x00, 0, 0},
{0x3811, 0x10, 0, 0}, {0x3812, 0x00, 0, 0}, {0x3813, 0x06, 0, 0},
{0x3618, 0x00, 0, 0}, {0x3612, 0x29, 0, 0}, {0x3708, 0x64, 0, 0},
@@ -401,8 +395,7 @@ static const struct reg_value
ov5640_setting_30fps_XGA_1024_768[] = {
{0x4001, 0x02, 0, 0}, {0x4004, 0x02, 0, 0}, {0x4713, 0x03, 0, 0},
{0x4407, 0x04, 0, 0}, {0x460b, 0x35, 0, 0}, {0x460c, 0x22, 0, 0},
{0x3824, 0x02, 0, 0}, {0x5001, 0xa3, 0, 0}, {0x3503, 0x00, 0, 0},
- {0x3808, 0x04, 0, 0}, {0x3809, 0x00, 0, 0}, {0x380a, 0x03, 0, 0},
- {0x380b, 0x00, 0, 0}, {0x3035, 0x12, 0, 0},
+ {0x3035, 0x12, 0, 0},
};
static const struct reg_value ov5640_setting_15fps_XGA_1024_768[] = {
@@ -412,8 +405,6 @@ static const struct reg_value
ov5640_setting_15fps_XGA_1024_768[] = {
{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
{0x3802, 0x00, 0, 0}, {0x3803, 0x04, 0, 0}, {0x3804, 0x0a, 0, 0},
{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9b, 0, 0},
- {0x3808, 0x02, 0, 0}, {0x3809, 0x80, 0, 0}, {0x380a, 0x01, 0, 0},
- {0x380b, 0xe0, 0, 0},
{0x3810, 0x00, 0, 0},
{0x3811, 0x10, 0, 0}, {0x3812, 0x00, 0, 0}, {0x3813, 0x06, 0, 0},
{0x3618, 0x00, 0, 0}, {0x3612, 0x29, 0, 0}, {0x3708, 0x64, 0, 0},
@@ -423,8 +414,7 @@ static const struct reg_value
ov5640_setting_15fps_XGA_1024_768[] = {
{0x3a0d, 0x04, 0, 0}, {0x3a14, 0x03, 0, 0}, {0x3a15, 0xd8, 0, 0},
{0x4001, 0x02, 0, 0}, {0x4004, 0x02, 0, 0}, {0x4713, 0x03, 0, 0},
{0x4407, 0x04, 0, 0}, {0x460b, 0x35, 0, 0}, {0x460c, 0x22, 0, 0},
- {0x3824, 0x02, 0, 0}, {0x5001, 0xa3, 0, 0}, {0x3808, 0x04, 0, 0},
- {0x3809, 0x00, 0, 0}, {0x380a, 0x03, 0, 0}, {0x380b, 0x00, 0, 0},
+ {0x3824, 0x02, 0,
[PATCH 01/12] media: ov5640: Add auto-focus feature
From: Mylène Josserand
Add the auto-focus ENABLE/DISABLE feature as V4L2 control.
Disabled by default.
Signed-off-by: Mylène Josserand
Signed-off-by: Maxime Ripard
---
drivers/media/i2c/ov5640.c | 16 +++-
1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index e2dd352224c7..f5e867d47ab8 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -80,8 +80,9 @@
#define OV5640_REG_POLARITY_CTRL00 0x4740
#define OV5640_REG_MIPI_CTRL00 0x4800
#define OV5640_REG_DEBUG_MODE 0x4814
-#define OV5640_REG_ISP_FORMAT_MUX_CTRL 0x501f
+#define OV5640_REG_ISP_CTRL03 0x5003
#define OV5640_REG_PRE_ISP_TEST_SET1 0x503d
+#define OV5640_REG_ISP_FORMAT_MUX_CTRL 0x501f
#define OV5640_REG_SDE_CTRL0 0x5580
#define OV5640_REG_SDE_CTRL1 0x5581
#define OV5640_REG_SDE_CTRL3 0x5583
@@ -183,6 +184,7 @@ struct ov5640_ctrls {
struct v4l2_ctrl *auto_gain;
struct v4l2_ctrl *gain;
};
+ struct v4l2_ctrl *auto_focus;
struct v4l2_ctrl *brightness;
struct v4l2_ctrl *saturation;
struct v4l2_ctrl *contrast;
@@ -2094,6 +2096,12 @@ static int ov5640_set_ctrl_test_pattern(struct
ov5640_dev *sensor, int value)
0xa4, value ? 0xa4 : 0);
}
+static int ov5640_set_ctrl_focus(struct ov5640_dev *sensor, int value)
+{
+ return ov5640_mod_reg(sensor, OV5640_REG_ISP_CTRL03,
+ BIT(1), value ? BIT(1) : 0);
+}
+
static int ov5640_g_volatile_ctrl(struct v4l2_ctrl *ctrl)
{
struct v4l2_subdev *sd = ctrl_to_sd(ctrl);
@@ -2162,6 +2170,9 @@ static int ov5640_s_ctrl(struct v4l2_ctrl *ctrl)
case V4L2_CID_TEST_PATTERN:
ret = ov5640_set_ctrl_test_pattern(sensor, ctrl->val);
break;
+ case V4L2_CID_FOCUS_AUTO:
+ ret = ov5640_set_ctrl_focus(sensor, ctrl->val);
+ break;
default:
ret = -EINVAL;
break;
@@ -2224,6 +2235,9 @@ static int ov5640_init_controls(struct ov5640_dev *sensor)
ARRAY_SIZE(test_pattern_menu) - 1,
0, 0, test_pattern_menu);
+ ctrls->auto_focus = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_FOCUS_AUTO,
+ 0, 1, 1, 0);
+
if (hdl->error) {
ret = hdl->error;
goto free_ctrls;
--
2.14.3
[PATCH 02/12] media: ov5640: Add light frequency control
From: Mylène Josserand
Add the light frequency control to be able to set the frequency
to manual (50Hz or 60Hz) or auto.
Signed-off-by: Mylène Josserand
Signed-off-by: Maxime Ripard
---
drivers/media/i2c/ov5640.c | 24
1 file changed, 24 insertions(+)
diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index f5e867d47ab8..e6a23eb55c1d 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -186,6 +186,7 @@ struct ov5640_ctrls {
};
struct v4l2_ctrl *auto_focus;
struct v4l2_ctrl *brightness;
+ struct v4l2_ctrl *light_freq;
struct v4l2_ctrl *saturation;
struct v4l2_ctrl *contrast;
struct v4l2_ctrl *hue;
@@ -2102,6 +2103,21 @@ static int ov5640_set_ctrl_focus(struct ov5640_dev
*sensor, int value)
BIT(1), value ? BIT(1) : 0);
}
+static int ov5640_set_ctl_light_freq(struct ov5640_dev *sensor, int value)
+{
+ int ret;
+
+ ret = ov5640_mod_reg(sensor, OV5640_REG_HZ5060_CTRL01, BIT(7),
+(value == V4L2_CID_POWER_LINE_FREQUENCY_AUTO) ?
+0: BIT(7));
+ if (ret)
+ return ret;
+
+ return ov5640_mod_reg(sensor, OV5640_REG_HZ5060_CTRL00, BIT(2),
+ (value == V4L2_CID_POWER_LINE_FREQUENCY_50HZ) ?
+ BIT(2): 0);
+}
+
static int ov5640_g_volatile_ctrl(struct v4l2_ctrl *ctrl)
{
struct v4l2_subdev *sd = ctrl_to_sd(ctrl);
@@ -2173,6 +2189,9 @@ static int ov5640_s_ctrl(struct v4l2_ctrl *ctrl)
case V4L2_CID_FOCUS_AUTO:
ret = ov5640_set_ctrl_focus(sensor, ctrl->val);
break;
+ case V4L2_CID_POWER_LINE_FREQUENCY:
+ ret = ov5640_set_ctl_light_freq(sensor, ctrl->val);
+ break;
default:
ret = -EINVAL;
break;
@@ -2237,6 +2256,11 @@ static int ov5640_init_controls(struct ov5640_dev
*sensor)
ctrls->auto_focus = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_FOCUS_AUTO,
0, 1, 1, 0);
+ ctrls->light_freq =
+ v4l2_ctrl_new_std_menu(hdl, ops,
+ V4L2_CID_POWER_LINE_FREQUENCY,
+ V4L2_CID_POWER_LINE_FREQUENCY_AUTO, 0,
+ V4L2_CID_POWER_LINE_FREQUENCY_50HZ);
if (hdl->error) {
ret = hdl->error;
--
2.14.3
[PATCH 10/12] media: ov5640: Enhance FPS handling
Now that we have moved the clock generation logic out of the bytes array,
these arrays are identical between the 15fps and 30fps variants.
Remove the duplicate entries, and convert the code accordingly.
Signed-off-by: Maxime Ripard
---
drivers/media/i2c/ov5640.c | 312 -
1 file changed, 56 insertions(+), 256 deletions(-)
diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index bdf378d80e07..5510a19281a4 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -344,66 +344,7 @@ static const struct reg_value
ov5640_init_setting_30fps_VGA[] = {
{0x3a1f, 0x14, 0, 0}, {0x3008, 0x02, 0, 0}, {0x3c00, 0x04, 0, 300},
};
-static const struct reg_value ov5640_setting_30fps_VGA_640_480[] = {
-
- {0x3c07, 0x08, 0, 0},
- {0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
- {0x3820, 0x41, 0, 0}, {0x3821, 0x07, 0, 0}, {0x3814, 0x31, 0, 0},
- {0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
- {0x3802, 0x00, 0, 0}, {0x3803, 0x04, 0, 0}, {0x3804, 0x0a, 0, 0},
- {0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9b, 0, 0},
- {0x3810, 0x00, 0, 0},
- {0x3811, 0x10, 0, 0}, {0x3812, 0x00, 0, 0}, {0x3813, 0x06, 0, 0},
- {0x3618, 0x00, 0, 0}, {0x3612, 0x29, 0, 0}, {0x3708, 0x64, 0, 0},
- {0x3709, 0x52, 0, 0}, {0x370c, 0x03, 0, 0}, {0x3a02, 0x03, 0, 0},
- {0x3a03, 0xd8, 0, 0}, {0x3a08, 0x01, 0, 0}, {0x3a09, 0x0e, 0, 0},
- {0x3a0a, 0x00, 0, 0}, {0x3a0b, 0xf6, 0, 0}, {0x3a0e, 0x03, 0, 0},
- {0x3a0d, 0x04, 0, 0}, {0x3a14, 0x03, 0, 0}, {0x3a15, 0xd8, 0, 0},
- {0x4001, 0x02, 0, 0}, {0x4004, 0x02, 0, 0}, {0x4713, 0x03, 0, 0},
- {0x4407, 0x04, 0, 0}, {0x460b, 0x35, 0, 0}, {0x460c, 0x22, 0, 0},
- {0x3824, 0x02, 0, 0}, {0x5001, 0xa3, 0, 0}, {0x3503, 0x00, 0, 0},
-};
-
-static const struct reg_value ov5640_setting_15fps_VGA_640_480[] = {
- {0x3c07, 0x08, 0, 0},
- {0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
- {0x3820, 0x41, 0, 0}, {0x3821, 0x07, 0, 0}, {0x3814, 0x31, 0, 0},
- {0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
- {0x3802, 0x00, 0, 0}, {0x3803, 0x04, 0, 0}, {0x3804, 0x0a, 0, 0},
- {0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9b, 0, 0},
- {0x3810, 0x00, 0, 0},
- {0x3811, 0x10, 0, 0}, {0x3812, 0x00, 0, 0}, {0x3813, 0x06, 0, 0},
- {0x3618, 0x00, 0, 0}, {0x3612, 0x29, 0, 0}, {0x3708, 0x64, 0, 0},
- {0x3709, 0x52, 0, 0}, {0x370c, 0x03, 0, 0}, {0x3a02, 0x03, 0, 0},
- {0x3a03, 0xd8, 0, 0}, {0x3a08, 0x01, 0, 0}, {0x3a09, 0x27, 0, 0},
- {0x3a0a, 0x00, 0, 0}, {0x3a0b, 0xf6, 0, 0}, {0x3a0e, 0x03, 0, 0},
- {0x3a0d, 0x04, 0, 0}, {0x3a14, 0x03, 0, 0}, {0x3a15, 0xd8, 0, 0},
- {0x4001, 0x02, 0, 0}, {0x4004, 0x02, 0, 0}, {0x4713, 0x03, 0, 0},
- {0x4407, 0x04, 0, 0}, {0x460b, 0x35, 0, 0}, {0x460c, 0x22, 0, 0},
- {0x3824, 0x02, 0, 0}, {0x5001, 0xa3, 0, 0},
-};
-
-static const struct reg_value ov5640_setting_30fps_XGA_1024_768[] = {
-
- {0x3c07, 0x08, 0, 0},
- {0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
- {0x3820, 0x41, 0, 0}, {0x3821, 0x07, 0, 0}, {0x3814, 0x31, 0, 0},
- {0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
- {0x3802, 0x00, 0, 0}, {0x3803, 0x04, 0, 0}, {0x3804, 0x0a, 0, 0},
- {0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9b, 0, 0},
- {0x3810, 0x00, 0, 0},
- {0x3811, 0x10, 0, 0}, {0x3812, 0x00, 0, 0}, {0x3813, 0x06, 0, 0},
- {0x3618, 0x00, 0, 0}, {0x3612, 0x29, 0, 0}, {0x3708, 0x64, 0, 0},
- {0x3709, 0x52, 0, 0}, {0x370c, 0x03, 0, 0}, {0x3a02, 0x03, 0, 0},
- {0x3a03, 0xd8, 0, 0}, {0x3a08, 0x01, 0, 0}, {0x3a09, 0x0e, 0, 0},
- {0x3a0a, 0x00, 0, 0}, {0x3a0b, 0xf6, 0, 0}, {0x3a0e, 0x03, 0, 0},
- {0x3a0d, 0x04, 0, 0}, {0x3a14, 0x03, 0, 0}, {0x3a15, 0xd8, 0, 0},
- {0x4001, 0x02, 0, 0}, {0x4004, 0x02, 0, 0}, {0x4713, 0x03, 0, 0},
- {0x4407, 0x04, 0, 0}, {0x460b, 0x35, 0, 0}, {0x460c, 0x22, 0, 0},
- {0x3824, 0x02, 0, 0}, {0x5001, 0xa3, 0, 0}, {0x3503, 0x00, 0, 0},
-};
-
-static const struct reg_value ov5640_setting_15fps_XGA_1024_768[] = {
+static const struct reg_value ov5640_setting_VGA_640_480[] = {
{0x3c07, 0x08, 0, 0},
{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
{0x3820, 0x41, 0, 0}, {0x3821, 0x07, 0, 0}, {0x3814, 0x31, 0, 0},
@@ -422,7 +363,7 @@ static const struct reg_value
ov5640_setting_15fps_XGA_1024_768[] = {
{0x3824, 0x02, 0, 0}, {0x5001, 0xa3, 0, 0},
};
-static const struct reg_value ov5640_setting_30fps_QVGA_320_240[] = {
+static const struct reg_value ov5640_setting_XGA_1024_768[] = {
{0x3c07, 0x08, 0, 0},
{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
{0x3820, 0x41, 0, 0}, {0x3821, 0x07, 0, 0}, {0x3814, 0x31, 0, 0},
@@ -441
[PATCH 06/12] media: ov5640: Add horizontal and vertical totals
All the initialization arrays are changing the horizontal and vertical
totals for some value.
In order to clean up the driver, and since we're going to need that value
later on, let's introduce in the ov5640_mode_info structure the horizontal
and vertical total sizes, and move these out of the bytes array.
Signed-off-by: Maxime Ripard
---
drivers/media/i2c/ov5640.c | 156 -
1 file changed, 97 insertions(+), 59 deletions(-)
diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index 8c04f2880715..443b167bcd20 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -167,7 +167,9 @@ struct ov5640_mode_info {
enum ov5640_mode_id id;
enum ov5640_downsize_mode dn_mode;
u32 hact;
+ u32 htot;
u32 vact;
+ u32 vtot;
const struct reg_value *reg_data;
u32 reg_data_size;
};
@@ -270,8 +272,8 @@ static const struct reg_value
ov5640_init_setting_30fps_VGA[] = {
{0x3802, 0x00, 0, 0}, {0x3803, 0x04, 0, 0}, {0x3804, 0x0a, 0, 0},
{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9b, 0, 0},
{0x3808, 0x02, 0, 0}, {0x3809, 0x80, 0, 0}, {0x380a, 0x01, 0, 0},
- {0x380b, 0xe0, 0, 0}, {0x380c, 0x07, 0, 0}, {0x380d, 0x68, 0, 0},
- {0x380e, 0x03, 0, 0}, {0x380f, 0xd8, 0, 0}, {0x3810, 0x00, 0, 0},
+ {0x380b, 0xe0, 0, 0},
+ {0x3810, 0x00, 0, 0},
{0x3811, 0x10, 0, 0}, {0x3812, 0x00, 0, 0}, {0x3813, 0x06, 0, 0},
{0x3618, 0x00, 0, 0}, {0x3612, 0x29, 0, 0}, {0x3708, 0x64, 0, 0},
{0x3709, 0x52, 0, 0}, {0x370c, 0x03, 0, 0}, {0x3a02, 0x03, 0, 0},
@@ -345,8 +347,8 @@ static const struct reg_value
ov5640_setting_30fps_VGA_640_480[] = {
{0x3802, 0x00, 0, 0}, {0x3803, 0x04, 0, 0}, {0x3804, 0x0a, 0, 0},
{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9b, 0, 0},
{0x3808, 0x02, 0, 0}, {0x3809, 0x80, 0, 0}, {0x380a, 0x01, 0, 0},
- {0x380b, 0xe0, 0, 0}, {0x380c, 0x07, 0, 0}, {0x380d, 0x68, 0, 0},
- {0x380e, 0x04, 0, 0}, {0x380f, 0x38, 0, 0}, {0x3810, 0x00, 0, 0},
+ {0x380b, 0xe0, 0, 0},
+ {0x3810, 0x00, 0, 0},
{0x3811, 0x10, 0, 0}, {0x3812, 0x00, 0, 0}, {0x3813, 0x06, 0, 0},
{0x3618, 0x00, 0, 0}, {0x3612, 0x29, 0, 0}, {0x3708, 0x64, 0, 0},
{0x3709, 0x52, 0, 0}, {0x370c, 0x03, 0, 0}, {0x3a02, 0x03, 0, 0},
@@ -366,8 +368,8 @@ static const struct reg_value
ov5640_setting_15fps_VGA_640_480[] = {
{0x3802, 0x00, 0, 0}, {0x3803, 0x04, 0, 0}, {0x3804, 0x0a, 0, 0},
{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9b, 0, 0},
{0x3808, 0x02, 0, 0}, {0x3809, 0x80, 0, 0}, {0x380a, 0x01, 0, 0},
- {0x380b, 0xe0, 0, 0}, {0x380c, 0x07, 0, 0}, {0x380d, 0x68, 0, 0},
- {0x380e, 0x03, 0, 0}, {0x380f, 0xd8, 0, 0}, {0x3810, 0x00, 0, 0},
+ {0x380b, 0xe0, 0, 0},
+ {0x3810, 0x00, 0, 0},
{0x3811, 0x10, 0, 0}, {0x3812, 0x00, 0, 0}, {0x3813, 0x06, 0, 0},
{0x3618, 0x00, 0, 0}, {0x3612, 0x29, 0, 0}, {0x3708, 0x64, 0, 0},
{0x3709, 0x52, 0, 0}, {0x370c, 0x03, 0, 0}, {0x3a02, 0x03, 0, 0},
@@ -388,8 +390,8 @@ static const struct reg_value
ov5640_setting_30fps_XGA_1024_768[] = {
{0x3802, 0x00, 0, 0}, {0x3803, 0x04, 0, 0}, {0x3804, 0x0a, 0, 0},
{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9b, 0, 0},
{0x3808, 0x02, 0, 0}, {0x3809, 0x80, 0, 0}, {0x380a, 0x01, 0, 0},
- {0x380b, 0xe0, 0, 0}, {0x380c, 0x07, 0, 0}, {0x380d, 0x68, 0, 0},
- {0x380e, 0x04, 0, 0}, {0x380f, 0x38, 0, 0}, {0x3810, 0x00, 0, 0},
+ {0x380b, 0xe0, 0, 0},
+ {0x3810, 0x00, 0, 0},
{0x3811, 0x10, 0, 0}, {0x3812, 0x00, 0, 0}, {0x3813, 0x06, 0, 0},
{0x3618, 0x00, 0, 0}, {0x3612, 0x29, 0, 0}, {0x3708, 0x64, 0, 0},
{0x3709, 0x52, 0, 0}, {0x370c, 0x03, 0, 0}, {0x3a02, 0x03, 0, 0},
@@ -411,8 +413,8 @@ static const struct reg_value
ov5640_setting_15fps_XGA_1024_768[] = {
{0x3802, 0x00, 0, 0}, {0x3803, 0x04, 0, 0}, {0x3804, 0x0a, 0, 0},
{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9b, 0, 0},
{0x3808, 0x02, 0, 0}, {0x3809, 0x80, 0, 0}, {0x380a, 0x01, 0, 0},
- {0x380b, 0xe0, 0, 0}, {0x380c, 0x07, 0, 0}, {0x380d, 0x68, 0, 0},
- {0x380e, 0x03, 0, 0}, {0x380f, 0xd8, 0, 0}, {0x3810, 0x00, 0, 0},
+ {0x380b, 0xe0, 0, 0},
+ {0x3810, 0x00, 0, 0},
{0x3811, 0x10, 0, 0}, {0x3812, 0x00, 0, 0}, {0x3813, 0x06, 0, 0},
{0x3618, 0x00, 0, 0}, {0x3612, 0x29, 0, 0}, {0x3708, 0x64, 0, 0},
{0x3709, 0x52, 0, 0}, {0x370c, 0x03, 0, 0}, {0x3a02, 0x03, 0, 0},
@@ -433,8 +435,8 @@ static const struct reg_value
ov5640_setting_30fps_QVGA_320_240[] = {
{0x3802, 0x00, 0, 0}, {0x3803, 0x04, 0, 0}, {0x3804, 0x0a, 0, 0},
{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9b, 0, 0},
{0x3808, 0x01, 0, 0}, {0x3809, 0x40, 0, 0}, {0x380a, 0x00, 0, 0},
- {0x380b, 0xf0, 0, 0}, {0x380c, 0x07, 0, 0}, {0x380d,
[PATCH 09/12] media: ov5640: Compute the clock rate at runtime
The clock rate, while hardcoded until now, is actually a function of the
resolution, framerate and bytes per pixel. Now that we have an algorithm to
adjust our clock rate, we can select it dynamically when we change the
mode.
This changes a bit the clock rate being used, with the following effect:
+--+--+--+--+-+-++---+
| Hact | Vact | Htot | Vtot | FPS | Hardcoded clock | Computed clock |
Deviation |
+--+--+--+--+-+-++---+
| 640 | 480 | 1896 | 1080 | 15 |5600 | 61430400 | 8.84 %
|
| 640 | 480 | 1896 | 1080 | 30 | 11200 | 122860800 | 8.84 %
|
| 1024 | 768 | 1896 | 1080 | 15 |5600 | 61430400 | 8.84 %
|
| 1024 | 768 | 1896 | 1080 | 30 | 11200 | 122860800 | 8.84 %
|
| 320 | 240 | 1896 | 984 | 15 |5600 | 55969920 | 0.05 %
|
| 320 | 240 | 1896 | 984 | 30 | 11200 | 111939840 | 0.05 %
|
| 176 | 144 | 1896 | 984 | 15 |5600 | 55969920 | 0.05 %
|
| 176 | 144 | 1896 | 984 | 30 | 11200 | 111939840 | 0.05 %
|
| 720 | 480 | 1896 | 984 | 15 |5600 | 55969920 | 0.05 %
|
| 720 | 480 | 1896 | 984 | 30 | 11200 | 111939840 | 0.05 %
|
| 720 | 576 | 1896 | 984 | 15 |5600 | 55969920 | 0.05 %
|
| 720 | 576 | 1896 | 984 | 30 | 11200 | 111939840 | 0.05 %
|
| 1280 | 720 | 1892 | 740 | 15 |4200 | 42002400 | 0.01 %
|
| 1280 | 720 | 1892 | 740 | 30 |8400 | 84004800 | 0.01 %
|
| 1920 | 1080 | 2500 | 1120 | 15 |8400 | 8400 | 0.00 %
|
| 1920 | 1080 | 2500 | 1120 | 30 | 16800 | 16800 | 0.00 %
|
| 2592 | 1944 | 2844 | 1944 | 15 |8400 | 165862080 | 49.36 %
|
+--+--+--+--+-+-++---+
Only the 640x480, 1024x768 and 2592x1944 modes are significantly affected
by the new formula.
In this case, 640x480 and 1024x768 are actually fixed by this driver.
Indeed, the sensor was sending data at, for example, 27.33fps instead of
30fps. This is -9%, which is roughly what we're seeing in the array.
Testing these modes with the new clock setup actually fix that error, and
data are now sent at around 30fps.
2592x1944, on the other hand, is probably due to the fact that this mode
can only be used using MIPI-CSI2, in a two lane mode. This would have to be
tested though.
Signed-off-by: Maxime Ripard
---
drivers/media/i2c/ov5640.c | 41 +
1 file changed, 17 insertions(+), 24 deletions(-)
diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index 323cde27dd8b..bdf378d80e07 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -126,6 +126,12 @@ static const struct ov5640_pixfmt ov5640_formats[] = {
{ MEDIA_BUS_FMT_RGB565_2X8_BE, V4L2_COLORSPACE_SRGB, },
};
+/*
+ * FIXME: If we ever have something else, we'll obviously need to have
+ * something smarter.
+ */
+#define OV5640_FORMATS_BPP 2
+
/*
* FIXME: remove this when a subdev API becomes available
* to set the MIPI CSI-2 virtual channel.
@@ -172,7 +178,6 @@ struct ov5640_mode_info {
u32 htot;
u32 vact;
u32 vtot;
- u32 clock;
const struct reg_value *reg_data;
u32 reg_data_size;
};
@@ -696,7 +701,6 @@ static const struct reg_value
ov5640_setting_15fps_QSXGA_2592_1944[] = {
/* power-on sensor init reg table */
static const struct ov5640_mode_info ov5640_mode_init_data = {
0, SUBSAMPLING, 640, 1896, 480, 984,
- 11200,
ov5640_init_setting_30fps_VGA,
ARRAY_SIZE(ov5640_init_setting_30fps_VGA),
};
@@ -706,91 +710,74 @@ ov5640_mode_data[OV5640_NUM_FRAMERATES][OV5640_NUM_MODES]
= {
{
{OV5640_MODE_QCIF_176_144, SUBSAMPLING,
176, 1896, 144, 984,
-5600,
ov5640_setting_15fps_QCIF_176_144,
ARRAY_SIZE(ov5640_setting_15fps_QCIF_176_144)},
{OV5640_MODE_QVGA_320_240, SUBSAMPLING,
320, 1896, 240, 984,
-5600,
ov5640_setting_15fps_QVGA_320_240,
ARRAY_SIZE(ov5640_setting_15fps_QVGA_320_240)},
{OV5640_MODE_VGA_640_480, SUBSAMPLING,
640, 1896, 480, 1080,
-5600,
ov5640_setting_15fps_VGA_640_480,
ARRAY_SIZE(ov5640_setting_15fps_VGA_640_480)},
{OV5640_MODE_NTSC_720_480, SUBSAMPLING,
720, 1896, 480, 984,
-5600,
ov5640_setting_15fps_NTSC_720_480,
ARRAY_SIZE(ov5640_setting_15fps_NTSC_720_480)},
{OV5640_MODE_PAL_72
[PATCH 08/12] media: ov5640: Adjust the clock based on the expected rate
The clock structure for the PCLK is quite obscure in the documentation, and
was hardcoded through the bytes array of each and every mode.
This is troublesome, since we cannot adjust it at runtime based on other
parameters (such as the number of bytes per pixel), and we can't support
either framerates that have not been used by the various vendors, since we
don't have the needed initialization sequence.
We can however understand how the clock tree works, and then implement some
functions to derive the various parameters from a given rate. And now that
those parameters are calculated at runtime, we can remove them from the
initialization sequence.
The modes also gained a new parameter which is the clock that they are
running at, from the register writes they were doing, so for now the switch
to the new algorithm should be transparent.
Signed-off-by: Maxime Ripard
---
drivers/media/i2c/ov5640.c | 316 +
1 file changed, 291 insertions(+), 25 deletions(-)
diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index 0eeb1667bbe7..323cde27dd8b 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -172,6 +172,7 @@ struct ov5640_mode_info {
u32 htot;
u32 vact;
u32 vtot;
+ u32 clock;
const struct reg_value *reg_data;
u32 reg_data_size;
};
@@ -255,8 +256,8 @@ static const struct reg_value
ov5640_init_setting_30fps_VGA[] = {
{0x3103, 0x11, 0, 0}, {0x3008, 0x82, 0, 5}, {0x3008, 0x42, 0, 0},
{0x3103, 0x03, 0, 0}, {0x3017, 0x00, 0, 0}, {0x3018, 0x00, 0, 0},
- {0x3034, 0x18, 0, 0}, {0x3035, 0x14, 0, 0}, {0x3036, 0x38, 0, 0},
- {0x3037, 0x13, 0, 0}, {0x3630, 0x36, 0, 0},
+ {0x3034, 0x18, 0, 0},
+ {0x3630, 0x36, 0, 0},
{0x3631, 0x0e, 0, 0}, {0x3632, 0xe2, 0, 0}, {0x3633, 0x12, 0, 0},
{0x3621, 0xe0, 0, 0}, {0x3704, 0xa0, 0, 0}, {0x3703, 0x5a, 0, 0},
{0x3715, 0x78, 0, 0}, {0x3717, 0x01, 0, 0}, {0x370b, 0x60, 0, 0},
@@ -340,7 +341,7 @@ static const struct reg_value
ov5640_init_setting_30fps_VGA[] = {
static const struct reg_value ov5640_setting_30fps_VGA_640_480[] = {
- {0x3035, 0x14, 0, 0}, {0x3036, 0x38, 0, 0}, {0x3c07, 0x08, 0, 0},
+ {0x3c07, 0x08, 0, 0},
{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
{0x3820, 0x41, 0, 0}, {0x3821, 0x07, 0, 0}, {0x3814, 0x31, 0, 0},
{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
@@ -359,7 +360,7 @@ static const struct reg_value
ov5640_setting_30fps_VGA_640_480[] = {
};
static const struct reg_value ov5640_setting_15fps_VGA_640_480[] = {
- {0x3035, 0x22, 0, 0}, {0x3036, 0x38, 0, 0}, {0x3c07, 0x08, 0, 0},
+ {0x3c07, 0x08, 0, 0},
{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
{0x3820, 0x41, 0, 0}, {0x3821, 0x07, 0, 0}, {0x3814, 0x31, 0, 0},
{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
@@ -379,7 +380,7 @@ static const struct reg_value
ov5640_setting_15fps_VGA_640_480[] = {
static const struct reg_value ov5640_setting_30fps_XGA_1024_768[] = {
- {0x3035, 0x14, 0, 0}, {0x3036, 0x38, 0, 0}, {0x3c07, 0x08, 0, 0},
+ {0x3c07, 0x08, 0, 0},
{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
{0x3820, 0x41, 0, 0}, {0x3821, 0x07, 0, 0}, {0x3814, 0x31, 0, 0},
{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
@@ -395,11 +396,10 @@ static const struct reg_value
ov5640_setting_30fps_XGA_1024_768[] = {
{0x4001, 0x02, 0, 0}, {0x4004, 0x02, 0, 0}, {0x4713, 0x03, 0, 0},
{0x4407, 0x04, 0, 0}, {0x460b, 0x35, 0, 0}, {0x460c, 0x22, 0, 0},
{0x3824, 0x02, 0, 0}, {0x5001, 0xa3, 0, 0}, {0x3503, 0x00, 0, 0},
- {0x3035, 0x12, 0, 0},
};
static const struct reg_value ov5640_setting_15fps_XGA_1024_768[] = {
- {0x3035, 0x22, 0, 0}, {0x3036, 0x38, 0, 0}, {0x3c07, 0x08, 0, 0},
+ {0x3c07, 0x08, 0, 0},
{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
{0x3820, 0x41, 0, 0}, {0x3821, 0x07, 0, 0}, {0x3814, 0x31, 0, 0},
{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
@@ -418,7 +418,7 @@ static const struct reg_value
ov5640_setting_15fps_XGA_1024_768[] = {
};
static const struct reg_value ov5640_setting_30fps_QVGA_320_240[] = {
- {0x3035, 0x14, 0, 0}, {0x3036, 0x38, 0, 0}, {0x3c07, 0x08, 0, 0},
+ {0x3c07, 0x08, 0, 0},
{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
{0x3820, 0x41, 0, 0}, {0x3821, 0x07, 0, 0}, {0x3814, 0x31, 0, 0},
{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
@@ -437,7 +437,7 @@ static const struct reg_value
ov5640_setting_30fps_QVGA_320_240[] = {
};
static const struct reg_value ov5640_setting_15fps_QVGA_320_240[] = {
- {0x3035, 0x22, 0, 0}, {0x3036, 0x38, 0, 0}, {0x3c07, 0x08, 0, 0},
+ {0x3c07,
[PATCH 12/12] media: ov5640: Remove duplicate auto-exposure setup
The autoexposure setup in the 1080p init array is redundant with the
default value of the sensor.
Remove it.
Signed-off-by: Maxime Ripard
---
drivers/media/i2c/ov5640.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index 03838f42fb82..7c68c7013324 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -508,7 +508,7 @@ static const struct reg_value
ov5640_setting_1080P_1920_1080[] = {
{0x3a0e, 0x03, 0, 0}, {0x3a0d, 0x04, 0, 0}, {0x3a14, 0x04, 0, 0},
{0x3a15, 0x60, 0, 0}, {0x4713, 0x02, 0, 0}, {0x4407, 0x04, 0, 0},
{0x460b, 0x37, 0, 0}, {0x460c, 0x20, 0, 0}, {0x3824, 0x04, 0, 0},
- {0x4005, 0x1a, 0, 0}, {0x3008, 0x02, 0, 0}, {0x3503, 0, 0, 0},
+ {0x4005, 0x1a, 0, 0}, {0x3008, 0x02, 0, 0},
};
static const struct reg_value ov5640_setting_QSXGA_2592_1944[] = {
--
2.14.3
[PATCH 11/12] media: ov5640: Add 60 fps support
Now that we have everything in place to compute the clock rate at runtime,
we can enable the 60fps framerate for the mode we tested it with.
Signed-off-by: Maxime Ripard
---
drivers/media/i2c/ov5640.c | 20 +++-
1 file changed, 15 insertions(+), 5 deletions(-)
diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index 5510a19281a4..03838f42fb82 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -111,6 +111,7 @@ enum ov5640_mode_id {
enum ov5640_frame_rate {
OV5640_15_FPS = 0,
OV5640_30_FPS,
+ OV5640_60_FPS,
OV5640_NUM_FRAMERATES,
};
@@ -144,6 +145,7 @@ MODULE_PARM_DESC(virtual_channel,
static const int ov5640_framerates[] = {
[OV5640_15_FPS] = 15,
[OV5640_30_FPS] = 30,
+ [OV5640_60_FPS] = 60,
};
/* regulator supplies */
@@ -1447,6 +1449,11 @@ ov5640_find_mode(struct ov5640_dev *sensor, enum
ov5640_frame_rate fr,
fr != OV5640_15_FPS)
return NULL;
+ /* Only 640x480 can operate at 60fps (for now) */
+ if (fr == OV5640_60_FPS &&
+ width != 640 && height != 480)
+ return NULL;
+
return mode;
}
@@ -1875,12 +1882,12 @@ static int ov5640_try_frame_interval(struct ov5640_dev
*sensor,
int ret;
minfps = ov5640_framerates[OV5640_15_FPS];
- maxfps = ov5640_framerates[OV5640_30_FPS];
+ maxfps = ov5640_framerates[OV5640_60_FPS];
if (fi->numerator == 0) {
fi->denominator = maxfps;
fi->numerator = 1;
- return OV5640_30_FPS;
+ return OV5640_60_FPS;
}
fps = DIV_ROUND_CLOSEST(fi->denominator, fi->numerator);
@@ -1892,10 +1899,13 @@ static int ov5640_try_frame_interval(struct ov5640_dev
*sensor,
fi->denominator = minfps;
else if (2 * fps >= 2 * minfps + (maxfps - minfps))
fi->denominator = maxfps;
- else
- fi->denominator = minfps;
- ret = (fi->denominator == minfps) ? OV5640_15_FPS : OV5640_30_FPS;
+ if (fi->denominator == minfps)
+ ret = OV5640_15_FPS;
+ else if (fi->denominator == maxfps)
+ ret = OV5640_60_FPS;
+ else
+ ret = OV5640_30_FPS;
mode = ov5640_find_mode(sensor, ret, width, height, false);
return mode ? ret : -EINVAL;
--
2.14.3
[PATCH v2 00/11] media: ov772x/tw9910 cleanup
Hi Mauro,
as I had one more patch to add to the series, I have now re-based it on top
of Joe's changes, which were based on top of yours already part of media-tree
master branch.
Please apply on top of:
commit bc3c49d6bbfb ("media: tw9910: Miscellaneous neatening")
commit 71c07c61b340 ("media: tw9910: Whitespace alignment")
commit ae24b8a1d5f9 ("media: tw9910: solve coding style issues")
commit 2d595d14fe8b ("media: ov772x: fix whitespace issues")
Thanks
j
v1 -> v2:
- Rebased on top of Joe's cleanup patches: 2 patches squashed
- Add patch 12/12
Jacopo Mondi (11):
media: tw9910: Re-order variables declaration
media: tw9910: Re-organize in-code comments
media: tw9910: Mixed style fixes
media: tw9910: Sort includes alphabetically
media: tw9910: Replace msleep(1) with usleep_range
media: ov772x: Align function parameters
media: ov772x: Re-organize in-code comments
media: ov772x: Empty line before end-of-function return
media: ov772x: Re-order variables declaration
media: ov772x: Replace msleep(1) with usleep_range
media: ov772x: Unregister async subdevice
drivers/media/i2c/ov772x.c | 65
drivers/media/i2c/tw9910.c | 83 ++
2 files changed, 61 insertions(+), 87 deletions(-)
--
2.7.4
[PATCH v2 09/11] media: ov772x: Re-order variables declaration
Re-order variables declaration to respect 'reverse christmas tree'
ordering whenever possible.
Signed-off-by: Jacopo Mondi
---
drivers/media/i2c/ov772x.c | 17 +
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c
index 4f464ac..1fd6d4b 100644
--- a/drivers/media/i2c/ov772x.c
+++ b/drivers/media/i2c/ov772x.c
@@ -1098,8 +1098,8 @@ static int ov772x_set_fmt(struct v4l2_subdev *sd,
struct v4l2_subdev_pad_config *cfg,
struct v4l2_subdev_format *format)
{
- struct ov772x_priv *priv = to_ov772x(sd);
struct v4l2_mbus_framefmt *mf = &format->format;
+ struct ov772x_priv *priv = to_ov772x(sd);
const struct ov772x_color_format *cfmt;
const struct ov772x_win_size *win;
int ret;
@@ -1135,10 +1135,11 @@ static int ov772x_set_fmt(struct v4l2_subdev *sd,
static int ov772x_video_probe(struct ov772x_priv *priv)
{
- struct i2c_client *client = v4l2_get_subdevdata(&priv->subdev);
- u8 pid, ver;
- const char *devname;
- int ret;
+ struct i2c_client *client = v4l2_get_subdevdata(&priv->subdev);
+ const char *devname;
+ int ret;
+ u8 pid;
+ u8 ver;
ret = ov772x_s_power(&priv->subdev, 1);
if (ret < 0)
@@ -1246,9 +1247,9 @@ static const struct v4l2_subdev_ops ov772x_subdev_ops = {
static int ov772x_probe(struct i2c_client *client,
const struct i2c_device_id *did)
{
- struct ov772x_priv *priv;
- struct i2c_adapter *adapter = client->adapter;
- int ret;
+ struct i2c_adapter *adapter = client->adapter;
+ struct ov772x_priv *priv;
+ int ret;
if (!client->dev.platform_data) {
dev_err(&client->dev, "Missing ov772x platform data\n");
--
2.7.4
[PATCH v2 03/11] media: tw9910: Mixed style fixes
Two minor style fixes, align function parameter and remove un-necessary spaces. Signed-off-by: Jacopo Mondi --- drivers/media/i2c/tw9910.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/media/i2c/tw9910.c b/drivers/media/i2c/tw9910.c index 1c3c8f0..0232017 100644 --- a/drivers/media/i2c/tw9910.c +++ b/drivers/media/i2c/tw9910.c @@ -533,9 +533,9 @@ static int tw9910_s_std(struct v4l2_subdev *sd, v4l2_std_id norm) } if (!ret) ret = i2c_smbus_write_byte_data(client, CROP_HI, - ((vdelay >> 2) & 0xc0) | - ((vact >> 4) & 0x30) | - ((hdelay >> 6) & 0x0c) | + ((vdelay >> 2) & 0xc0) | + ((vact >> 4) & 0x30)| + ((hdelay >> 6) & 0x0c) | ((hact >> 8) & 0x03)); if (!ret) ret = i2c_smbus_write_byte_data(client, VDELAY_LO, @@ -953,7 +953,7 @@ static int tw9910_probe(struct i2c_client *client, if (!priv) return -ENOMEM; - priv->info = info; + priv->info = info; v4l2_i2c_subdev_init(&priv->subdev, client, &tw9910_subdev_ops); -- 2.7.4
[PATCH v2 01/11] media: tw9910: Re-order variables declaration
Re-order variables declaration to respect 'reverse christmas tree'
ordering whenever possible.
Signed-off-by: Jacopo Mondi
---
drivers/media/i2c/tw9910.c | 23 +++
1 file changed, 11 insertions(+), 12 deletions(-)
diff --git a/drivers/media/i2c/tw9910.c b/drivers/media/i2c/tw9910.c
index cc648de..3a5e307 100644
--- a/drivers/media/i2c/tw9910.c
+++ b/drivers/media/i2c/tw9910.c
@@ -406,9 +406,9 @@ static void tw9910_reset(struct i2c_client *client)
static int tw9910_power(struct i2c_client *client, int enable)
{
- int ret;
u8 acntl1;
u8 acntl2;
+ int ret;
if (enable) {
acntl1 = 0;
@@ -428,8 +428,8 @@ static int tw9910_power(struct i2c_client *client, int
enable)
static const struct tw9910_scale_ctrl *tw9910_select_norm(v4l2_std_id norm,
u32 width, u32 height)
{
- const struct tw9910_scale_ctrl *scale;
const struct tw9910_scale_ctrl *ret = NULL;
+ const struct tw9910_scale_ctrl *scale;
__u32 diff = 0x, tmp;
int size, i;
@@ -462,8 +462,8 @@ static int tw9910_s_stream(struct v4l2_subdev *sd, int
enable)
{
struct i2c_client *client = v4l2_get_subdevdata(sd);
struct tw9910_priv *priv = to_tw9910(client);
- u8 val;
int ret;
+ u8 val;
if (!enable) {
switch (priv->revision) {
@@ -512,10 +512,10 @@ static int tw9910_s_std(struct v4l2_subdev *sd,
v4l2_std_id norm)
{
struct i2c_client *client = v4l2_get_subdevdata(sd);
struct tw9910_priv *priv = to_tw9910(client);
- const unsigned int hact = 720;
const unsigned int hdelay = 15;
- unsigned int vact;
+ const unsigned int hact = 720;
unsigned int vdelay;
+ unsigned int vact;
int ret;
if (!(norm & (V4L2_STD_NTSC | V4L2_STD_PAL)))
@@ -760,8 +760,8 @@ static int tw9910_get_fmt(struct v4l2_subdev *sd,
struct v4l2_subdev_pad_config *cfg,
struct v4l2_subdev_format *format)
{
- struct v4l2_mbus_framefmt *mf = &format->format;
struct i2c_client *client = v4l2_get_subdevdata(sd);
+ struct v4l2_mbus_framefmt *mf = &format->format;
struct tw9910_priv *priv = to_tw9910(client);
if (format->pad)
@@ -813,8 +813,8 @@ static int tw9910_set_fmt(struct v4l2_subdev *sd,
struct v4l2_subdev_pad_config *cfg,
struct v4l2_subdev_format *format)
{
- struct v4l2_mbus_framefmt *mf = &format->format;
struct i2c_client *client = v4l2_get_subdevdata(sd);
+ struct v4l2_mbus_framefmt *mf = &format->format;
struct tw9910_priv *priv = to_tw9910(client);
const struct tw9910_scale_ctrl *scale;
@@ -852,8 +852,8 @@ static int tw9910_set_fmt(struct v4l2_subdev *sd,
static int tw9910_video_probe(struct i2c_client *client)
{
struct tw9910_priv *priv = to_tw9910(client);
- s32 id;
int ret;
+ s32 id;
/*
* tw9910 only use 8 or 16 bit bus width
@@ -949,10 +949,9 @@ static int tw9910_probe(struct i2c_client *client,
const struct i2c_device_id *did)
{
- struct tw9910_priv *priv;
- struct tw9910_video_info*info;
- struct i2c_adapter *adapter =
- to_i2c_adapter(client->dev.parent);
+ struct i2c_adapter *adapter = to_i2c_adapter(client->dev.parent);
+ struct tw9910_video_info *info;
+ struct tw9910_priv *priv;
int ret;
if (!client->dev.platform_data) {
--
2.7.4
[PATCH v2 02/11] media: tw9910: Re-organize in-code comments
A lot of comments that would fit a single line were spread on two or
more lines. Also fix capitalization and punctuation where appropriate.
Signed-off-by: Jacopo Mondi
---
drivers/media/i2c/tw9910.c | 44 +---
1 file changed, 13 insertions(+), 31 deletions(-)
diff --git a/drivers/media/i2c/tw9910.c b/drivers/media/i2c/tw9910.c
index 3a5e307..1c3c8f0 100644
--- a/drivers/media/i2c/tw9910.c
+++ b/drivers/media/i2c/tw9910.c
@@ -388,7 +388,7 @@ static int tw9910_set_hsync(struct i2c_client *client)
if (ret < 0)
return ret;
- /* So far only revisions 0 and 1 have been seen */
+ /* So far only revisions 0 and 1 have been seen. */
/* bit 2 - 0 */
if (priv->revision == 1)
ret = tw9910_mask_set(client, HSLOWCTL, 0x77,
@@ -652,21 +652,15 @@ static int tw9910_set_frame(struct v4l2_subdev *sd, u32
*width, u32 *height)
int ret = -EINVAL;
u8 val;
- /*
-* select suitable norm
-*/
+ /* Select suitable norm. */
priv->scale = tw9910_select_norm(priv->norm, *width, *height);
if (!priv->scale)
goto tw9910_set_fmt_error;
- /*
-* reset hardware
-*/
+ /* Reset hardware. */
tw9910_reset(client);
- /*
-* set bus width
-*/
+ /* Set bus width. */
val = 0x00;
if (priv->info->buswidth == 16)
val = LEN;
@@ -675,9 +669,7 @@ static int tw9910_set_frame(struct v4l2_subdev *sd, u32
*width, u32 *height)
if (ret < 0)
goto tw9910_set_fmt_error;
- /*
-* select MPOUT behavior
-*/
+ /* Select MPOUT behavior. */
switch (priv->info->mpout) {
case TW9910_MPO_VLOSS:
val = RTSEL_VLOSS; break;
@@ -703,16 +695,12 @@ static int tw9910_set_frame(struct v4l2_subdev *sd, u32
*width, u32 *height)
if (ret < 0)
goto tw9910_set_fmt_error;
- /*
-* set scale
-*/
+ /* Set scale. */
ret = tw9910_set_scale(client, priv->scale);
if (ret < 0)
goto tw9910_set_fmt_error;
- /*
-* set hsync
-*/
+ /* Set hsync. */
ret = tw9910_set_hsync(client);
if (ret < 0)
goto tw9910_set_fmt_error;
@@ -739,7 +727,7 @@ static int tw9910_get_selection(struct v4l2_subdev *sd,
if (sel->which != V4L2_SUBDEV_FORMAT_ACTIVE)
return -EINVAL;
- /* Only CROP, CROP_DEFAULT and CROP_BOUNDS are supported */
+ /* Only CROP, CROP_DEFAULT and CROP_BOUNDS are supported. */
if (sel->target > V4L2_SEL_TGT_CROP_BOUNDS)
return -EINVAL;
@@ -791,9 +779,7 @@ static int tw9910_s_fmt(struct v4l2_subdev *sd,
WARN_ON(mf->field != V4L2_FIELD_ANY &&
mf->field != V4L2_FIELD_INTERLACED_BT);
- /*
-* check color format
-*/
+ /* Check color format. */
if (mf->code != MEDIA_BUS_FMT_UYVY8_2X8)
return -EINVAL;
@@ -831,9 +817,7 @@ static int tw9910_set_fmt(struct v4l2_subdev *sd,
mf->code = MEDIA_BUS_FMT_UYVY8_2X8;
mf->colorspace = V4L2_COLORSPACE_SMPTE170M;
- /*
-* select suitable norm
-*/
+ /* Select suitable norm. */
scale = tw9910_select_norm(priv->norm, mf->width, mf->height);
if (!scale)
return -EINVAL;
@@ -855,9 +839,7 @@ static int tw9910_video_probe(struct i2c_client *client)
int ret;
s32 id;
- /*
-* tw9910 only use 8 or 16 bit bus width
-*/
+ /* TW9910 only use 8 or 16 bit bus width. */
if (priv->info->buswidth != 16 && priv->info->buswidth != 8) {
dev_err(&client->dev, "bus width error\n");
return -ENODEV;
@@ -868,8 +850,8 @@ static int tw9910_video_probe(struct i2c_client *client)
return ret;
/*
-* check and show Product ID
-* So far only revisions 0 and 1 have been seen
+* Check and show Product ID.
+* So far only revisions 0 and 1 have been seen.
*/
id = i2c_smbus_read_byte_data(client, ID);
priv->revision = GET_REV(id);
--
2.7.4
[PATCH v2 11/11] media: ov772x: Unregister async subdevice
As the media subdevice is registered with 'v4l2_async_register_subdev()' unregister it at module removal time with 'v4l2_async_unregister_subdev()' Signed-off-by: Jacopo Mondi --- drivers/media/i2c/ov772x.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c index 2d5281a..3d1ea58 100644 --- a/drivers/media/i2c/ov772x.c +++ b/drivers/media/i2c/ov772x.c @@ -1329,7 +1329,7 @@ static int ov772x_remove(struct i2c_client *client) clk_put(priv->clk); if (priv->pwdn_gpio) gpiod_put(priv->pwdn_gpio); - v4l2_device_unregister_subdev(&priv->subdev); + v4l2_async_unregister_subdev(&priv->subdev); v4l2_ctrl_handler_free(&priv->hdl); return 0; -- 2.7.4
[PATCH v2 05/11] media: tw9910: Replace msleep(1) with usleep_range
msleep() can sleep up to 20ms.
As suggested by Documentation/timers/timers_howto.txt replace it with
usleep_range() with up to 5ms delay.
Signed-off-by: Jacopo Mondi
---
drivers/media/i2c/tw9910.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/media/i2c/tw9910.c b/drivers/media/i2c/tw9910.c
index 9c12bda..6f2f1d7 100644
--- a/drivers/media/i2c/tw9910.c
+++ b/drivers/media/i2c/tw9910.c
@@ -401,7 +401,7 @@ static int tw9910_set_hsync(struct i2c_client *client)
static void tw9910_reset(struct i2c_client *client)
{
tw9910_mask_set(client, ACNTL1, SRESET, SRESET);
- msleep(1);
+ usleep_range(1000, 5000);
}
static int tw9910_power(struct i2c_client *client, int enable)
--
2.7.4
[PATCH v2 04/11] media: tw9910: Sort includes alphabetically
Sort include directives alphabetically to ease maintenance. Signed-off-by: Jacopo Mondi --- drivers/media/i2c/tw9910.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/media/i2c/tw9910.c b/drivers/media/i2c/tw9910.c index 0232017..9c12bda 100644 --- a/drivers/media/i2c/tw9910.c +++ b/drivers/media/i2c/tw9910.c @@ -16,13 +16,13 @@ */ #include +#include #include +#include #include +#include #include -#include #include -#include -#include #include #include -- 2.7.4
[PATCH v2 08/11] media: ov772x: Empty line before end-of-function return
Add an empty line before return at the end of functions. Signed-off-by: Jacopo Mondi --- drivers/media/i2c/ov772x.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c index 8849da1..4f464ac 100644 --- a/drivers/media/i2c/ov772x.c +++ b/drivers/media/i2c/ov772x.c @@ -1129,6 +1129,7 @@ static int ov772x_set_fmt(struct v4l2_subdev *sd, priv->win = win; priv->cfmt = cfmt; + return 0; } @@ -1172,6 +1173,7 @@ static int ov772x_video_probe(struct ov772x_priv *priv) done: ov772x_s_power(&priv->subdev, 0); + return ret; } @@ -1213,6 +1215,7 @@ static int ov772x_enum_mbus_code(struct v4l2_subdev *sd, return -EINVAL; code->code = ov772x_cfmts[code->index].code; + return 0; } @@ -1327,6 +1330,7 @@ static int ov772x_remove(struct i2c_client *client) gpiod_put(priv->pwdn_gpio); v4l2_device_unregister_subdev(&priv->subdev); v4l2_ctrl_handler_free(&priv->hdl); + return 0; } -- 2.7.4
[PATCH v2 07/11] media: ov772x: Re-organize in-code comments
A lot of comments that would fit a single line were spread on two or
more lines. Also fix capitalization and punctuation where appropriate.
Signed-off-by: Jacopo Mondi
---
drivers/media/i2c/ov772x.c | 32 ++--
1 file changed, 10 insertions(+), 22 deletions(-)
diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c
index a418455..8849da1 100644
--- a/drivers/media/i2c/ov772x.c
+++ b/drivers/media/i2c/ov772x.c
@@ -910,17 +910,13 @@ static int ov772x_set_params(struct ov772x_priv *priv,
int ret;
u8 val;
- /*
-* reset hardware
-*/
+ /* Reset hardware. */
ov772x_reset(client);
- /*
-* Edge Ctrl
-*/
+ /* Edge Ctrl. */
if (priv->info->edgectrl.strength & OV772X_MANUAL_EDGE_CTRL) {
/*
-* Manual Edge Control Mode
+* Manual Edge Control Mode.
*
* Edge auto strength bit is set by default.
* Remove it when manual mode.
@@ -944,9 +940,9 @@ static int ov772x_set_params(struct ov772x_priv *priv,
} else if (priv->info->edgectrl.upper > priv->info->edgectrl.lower) {
/*
-* Auto Edge Control Mode
+* Auto Edge Control Mode.
*
-* set upper and lower limit
+* Set upper and lower limit.
*/
ret = ov772x_mask_set(client,
EDGE_UPPER, OV772X_EDGE_UPPER_MASK,
@@ -961,7 +957,7 @@ static int ov772x_set_params(struct ov772x_priv *priv,
goto ov772x_set_fmt_error;
}
- /* Format and window size */
+ /* Format and window size. */
ret = ov772x_write(client, HSTART, win->rect.left >> 2);
if (ret < 0)
goto ov772x_set_fmt_error;
@@ -993,9 +989,7 @@ static int ov772x_set_params(struct ov772x_priv *priv,
if (ret < 0)
goto ov772x_set_fmt_error;
- /*
-* set DSP_CTRL3
-*/
+ /* Set DSP_CTRL3. */
val = cfmt->dsp3;
if (val) {
ret = ov772x_mask_set(client,
@@ -1011,9 +1005,7 @@ static int ov772x_set_params(struct ov772x_priv *priv,
goto ov772x_set_fmt_error;
}
- /*
-* set COM3
-*/
+ /* Set COM3. */
val = cfmt->com3;
if (priv->info->flags & OV772X_FLAG_VFLIP)
val |= VFLIP_IMG;
@@ -1041,9 +1033,7 @@ static int ov772x_set_params(struct ov772x_priv *priv,
if (ret < 0)
goto ov772x_set_fmt_error;
- /*
-* set COM8
-*/
+ /* Set COM8. */
if (priv->band_filter) {
ret = ov772x_mask_set(client, COM8, BNDF_ON_OFF, 1);
if (!ret)
@@ -1153,9 +1143,7 @@ static int ov772x_video_probe(struct ov772x_priv *priv)
if (ret < 0)
return ret;
- /*
-* check and show product ID and manufacturer ID
-*/
+ /* Check and show product ID and manufacturer ID. */
pid = ov772x_read(client, PID);
ver = ov772x_read(client, VER);
--
2.7.4
[PATCH v2 10/11] media: ov772x: Replace msleep(1) with usleep_range
msleep() can sleep up to 20ms. As suggested by Documentation/timers/timers_howto.txt replace it with usleep_range() with up to 5ms delay. Signed-off-by: Jacopo Mondi --- drivers/media/i2c/ov772x.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c index 1fd6d4b..2d5281a 100644 --- a/drivers/media/i2c/ov772x.c +++ b/drivers/media/i2c/ov772x.c @@ -574,7 +574,7 @@ static int ov772x_reset(struct i2c_client *client) if (ret < 0) return ret; - msleep(1); + usleep_range(1000, 5000); return ov772x_mask_set(client, COM2, SOFT_SLEEP_MODE, SOFT_SLEEP_MODE); } -- 2.7.4
[PATCH v2 06/11] media: ov772x: Align function parameters
Align all function parameters to first open brace when declaring
functions.
Signed-off-by: Jacopo Mondi
---
drivers/media/i2c/ov772x.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c
index 16665af..a418455 100644
--- a/drivers/media/i2c/ov772x.c
+++ b/drivers/media/i2c/ov772x.c
@@ -1064,7 +1064,7 @@ static int ov772x_set_params(struct ov772x_priv *priv,
static int ov772x_get_selection(struct v4l2_subdev *sd,
struct v4l2_subdev_pad_config *cfg,
- struct v4l2_subdev_selection *sel)
+ struct v4l2_subdev_selection *sel)
{
struct ov772x_priv *priv = to_ov772x(sd);
@@ -1087,7 +1087,7 @@ static int ov772x_get_selection(struct v4l2_subdev *sd,
static int ov772x_get_fmt(struct v4l2_subdev *sd,
struct v4l2_subdev_pad_config *cfg,
- struct v4l2_subdev_format *format)
+ struct v4l2_subdev_format *format)
{
struct v4l2_mbus_framefmt *mf = &format->format;
struct ov772x_priv *priv = to_ov772x(sd);
@@ -1106,7 +1106,7 @@ static int ov772x_get_fmt(struct v4l2_subdev *sd,
static int ov772x_set_fmt(struct v4l2_subdev *sd,
struct v4l2_subdev_pad_config *cfg,
- struct v4l2_subdev_format *format)
+ struct v4l2_subdev_format *format)
{
struct ov772x_priv *priv = to_ov772x(sd);
struct v4l2_mbus_framefmt *mf = &format->format;
@@ -1219,7 +1219,7 @@ static int ov772x_enum_frame_interval(struct v4l2_subdev
*sd,
static int ov772x_enum_mbus_code(struct v4l2_subdev *sd,
struct v4l2_subdev_pad_config *cfg,
- struct v4l2_subdev_mbus_code_enum *code)
+struct v4l2_subdev_mbus_code_enum *code)
{
if (code->pad || code->index >= ARRAY_SIZE(ov772x_cfmts))
return -EINVAL;
--
2.7.4
[PATCH v6] media: imx258: Add imx258 camera sensor driver
Add a V4L2 sub-device driver for the Sony IMX258 image sensor.
This is a camera sensor using the I2C bus for control and the
CSI-2 bus for data.
Signed-off-by: Jason Chen
Signed-off-by: Alan Chiang
---
since v2:
-- Update the streaming function to remove SW_STANDBY in the beginning.
-- Adjust the delay time from 1ms to 12ms before set stream-on register.
since v3:
-- fix the sd.entity to make code be compiled on the mainline kernel.
since v4:
-- Enabled AG, DG, and Exposure time control correctly.
since v5:
-- Sensor vendor provided a new setting to fix different CLK issue
-- Add one more resolution for 1048x780, used for VGA streaming
MAINTAINERS|7 +
drivers/media/i2c/Kconfig | 11 +
drivers/media/i2c/Makefile |1 +
drivers/media/i2c/imx258.c | 1341
4 files changed, 1360 insertions(+)
create mode 100644 drivers/media/i2c/imx258.c
diff --git a/MAINTAINERS b/MAINTAINERS
index a339bb5..9f75510 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -12646,6 +12646,13 @@ S: Maintained
F: drivers/ssb/
F: include/linux/ssb/
+SONY IMX258 SENSOR DRIVER
+M: Sakari Ailus
+L: linux-media@vger.kernel.org
+T: git git://linuxtv.org/media_tree.git
+S: Maintained
+F: drivers/media/i2c/imx258.c
+
SONY IMX274 SENSOR DRIVER
M: Leon Luo
L: linux-media@vger.kernel.org
diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index fd01842..bcd4bf1 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -565,6 +565,17 @@ config VIDEO_APTINA_PLL
config VIDEO_SMIAPP_PLL
tristate
+config VIDEO_IMX258
+ tristate "Sony IMX258 sensor support"
+ depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API
+ depends on MEDIA_CAMERA_SUPPORT
+ ---help---
+ This is a Video4Linux2 sensor-level driver for the Sony
+ IMX258 camera.
+
+ To compile this driver as a module, choose M here: the
+ module will be called imx258.
+
config VIDEO_IMX274
tristate "Sony IMX274 sensor support"
depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API
diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
index 1b62639..4bf7d00 100644
--- a/drivers/media/i2c/Makefile
+++ b/drivers/media/i2c/Makefile
@@ -94,6 +94,7 @@ obj-$(CONFIG_VIDEO_IR_I2C) += ir-kbd-i2c.o
obj-$(CONFIG_VIDEO_ML86V7667) += ml86v7667.o
obj-$(CONFIG_VIDEO_OV2659) += ov2659.o
obj-$(CONFIG_VIDEO_TC358743) += tc358743.o
+obj-$(CONFIG_VIDEO_IMX258) += imx258.o
obj-$(CONFIG_VIDEO_IMX274) += imx274.o
obj-$(CONFIG_SDR_MAX2175) += max2175.o
diff --git a/drivers/media/i2c/imx258.c b/drivers/media/i2c/imx258.c
new file mode 100644
index 000..0418edd
--- /dev/null
+++ b/drivers/media/i2c/imx258.c
@@ -0,0 +1,1341 @@
+// Copyright (C) 2018 Intel Corporation
+// SPDX-License-Identifier: GPL-2.0
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define IMX258_REG_VALUE_08BIT 1
+#define IMX258_REG_VALUE_16BIT 2
+#define IMX258_REG_VALUE_24BIT 3
+
+#define IMX258_REG_MODE_SELECT 0x0100
+#define IMX258_MODE_STANDBY0x00
+#define IMX258_MODE_STREAMING 0x01
+
+/* Chip ID */
+#define IMX258_REG_CHIP_ID 0x0016
+#define IMX258_CHIP_ID 0x0258
+
+/* V_TIMING internal */
+#define IMX258_REG_VTS 0x0340
+#define IMX258_VTS_30FPS 0x0c98
+#define IMX258_VTS_MAX 0x
+
+/*Frame Length Line*/
+#define IMX258_FLL_MIN 0x08a6
+#define IMX258_FLL_MAX 0x
+#define IMX258_FLL_STEP1
+#define IMX258_FLL_DEFAULT 0x0c98
+
+/* HBLANK control - read only */
+#define IMX258_PPL_650MHZ 5352
+#define IMX258_PPL_325MHZ 2676
+
+/* Exposure control */
+#define IMX258_REG_EXPOSURE0x0202
+#define IMX258_EXPOSURE_MIN4
+#define IMX258_EXPOSURE_STEP 1
+#define IMX258_EXPOSURE_DEFAULT0x640
+#define IMX258_EXPOSURE_MAX65535
+
+/* Analog gain control */
+#define IMX258_REG_ANALOG_GAIN 0x0204
+#define IMX258_ANA_GAIN_MIN0
+#define IMX258_ANA_GAIN_MAX0x1fff
+#define IMX258_ANA_GAIN_STEP 1
+#define IMX258_ANA_GAIN_DEFAULT0x0
+
+/* Digital gain control */
+#define IMX258_REG_GR_DIGITAL_GAIN 0x020e
+#define IMX258_REG_R_DIGITAL_GAIN 0x0210
+#define IMX258_REG_B_DIGITAL_GAIN 0x0212
+#define IMX258_REG_GB_DIGITAL_GAIN 0x0214
+#define IMX258_DGTL_GAIN_MIN 0
+#define IMX258_DGTL_GAIN_MAX 4096 /* Max = 0xFFF */
+#define IMX258_DGTL_GAIN_DEFAULT 1024
+#define IMX258_DGTL_GAIN_STEP 1
+
+/* Orientation */
+#define REG_MIRROR_FLIP_CONTROL0x0101
+#define REG_CONFIG_MIRROR_FLIP 0x03
+
+struct imx258_reg {
+ u16 address;
+ u8 val;
+};
+
+struct imx258_re
[PATCH v4] media: radio: Critical interrupt bugfix for si470x over i2c
Fixed si470x_start() disabling the interrupt signal, causing tune operations to never complete. This does not affect USB radios because they poll the registers instead of using the IRQ line. Stylistic and comment changes from v3. Signed-off-by: Douglas Fischer --- diff -uprN linux.orig/drivers/media/radio/si470x/radio-si470x-common.c linux/drivers/media/radio/si470x/radio-si470x-common.c --- linux.orig/drivers/media/radio/si470x/radio-si470x-common.c 2018-01-15 21:58:10.675620432 -0500 +++ linux/drivers/media/radio/si470x/radio-si470x-common.c 2018-03-02 10:22:05.490059995 -0500 @@ -377,8 +377,11 @@ int si470x_start(struct si470x_device *r goto done; /* sysconfig 1 */ - radio->registers[SYSCONFIG1] = - (de << 11) & SYSCONFIG1_DE; /* DE*/ + radio->registers[SYSCONFIG1] |= SYSCONFIG1_RDSIEN|SYSCONFIG1_STCIEN|SYSCONFIG1_RDS; + radio->registers[SYSCONFIG1] &= ~SYSCONFIG1_GPIO2; + radio->registers[SYSCONFIG1] |= SYSCONFIG1_GPIO2_INT; + if (de) + radio->registers[SYSCONFIG1] |= SYSCONFIG1_DE; retval = si470x_set_register(radio, SYSCONFIG1); if (retval < 0) goto done; diff -uprN linux.orig/drivers/media/radio/si470x/radio-si470x.h linux/drivers/media/radio/si470x/radio-si470x.h --- linux.orig/drivers/media/radio/si470x/radio-si470x.h2018-01-15 21:58:10.675620432 -0500 +++ linux/drivers/media/radio/si470x/radio-si470x.h 2018-03-02 10:22:05.497059995 -0500 @@ -79,6 +79,8 @@ #define SYSCONFIG1_BLNDADJ 0x00c0 /* bits 07..06: Stereo/Mono Blend Level Adjustment */ #define SYSCONFIG1_GPIO3 0x0030 /* bits 05..04: General Purpose I/O 3 */ #define SYSCONFIG1_GPIO2 0x000c /* bits 03..02: General Purpose I/O 2 */ +#define SYSCONFIG1_GPIO2_DIS 0x /* Disable GPIO 2 interrupt */ +#define SYSCONFIG1_GPIO2_INT 0x0004 /* Enable STC/RDS interrupt */ #define SYSCONFIG1_GPIO1 0x0003 /* bits 01..00: General Purpose I/O 1 */ #define SYSCONFIG2 5 /* System Configuration 2 */
Re: [PATCH v6] media: imx258: Add imx258 camera sensor driver
Hi Andy, Thanks for the patch. Let me post some comments inline. On Fri, Mar 2, 2018 at 11:55 PM, Andy Yeh wrote: > Add a V4L2 sub-device driver for the Sony IMX258 image sensor. > This is a camera sensor using the I2C bus for control and the > CSI-2 bus for data. > > Signed-off-by: Jason Chen > Signed-off-by: Alan Chiang > --- > since v2: > -- Update the streaming function to remove SW_STANDBY in the beginning. > -- Adjust the delay time from 1ms to 12ms before set stream-on register. > since v3: > -- fix the sd.entity to make code be compiled on the mainline kernel. > since v4: > -- Enabled AG, DG, and Exposure time control correctly. > since v5: > -- Sensor vendor provided a new setting to fix different CLK issue > -- Add one more resolution for 1048x780, used for VGA streaming > > MAINTAINERS|7 + > drivers/media/i2c/Kconfig | 11 + > drivers/media/i2c/Makefile |1 + > drivers/media/i2c/imx258.c | 1341 > > 4 files changed, 1360 insertions(+) > create mode 100644 drivers/media/i2c/imx258.c > > diff --git a/MAINTAINERS b/MAINTAINERS > index a339bb5..9f75510 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -12646,6 +12646,13 @@ S: Maintained > F: drivers/ssb/ > F: include/linux/ssb/ > > +SONY IMX258 SENSOR DRIVER > +M: Sakari Ailus > +L: linux-media@vger.kernel.org > +T: git git://linuxtv.org/media_tree.git > +S: Maintained > +F: drivers/media/i2c/imx258.c > + > SONY IMX274 SENSOR DRIVER > M: Leon Luo > L: linux-media@vger.kernel.org > diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig > index fd01842..bcd4bf1 100644 > --- a/drivers/media/i2c/Kconfig > +++ b/drivers/media/i2c/Kconfig > @@ -565,6 +565,17 @@ config VIDEO_APTINA_PLL > config VIDEO_SMIAPP_PLL > tristate > > +config VIDEO_IMX258 > + tristate "Sony IMX258 sensor support" > + depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API > + depends on MEDIA_CAMERA_SUPPORT > + ---help--- > + This is a Video4Linux2 sensor-level driver for the Sony > + IMX258 camera. > + > + To compile this driver as a module, choose M here: the > + module will be called imx258. > + > config VIDEO_IMX274 > tristate "Sony IMX274 sensor support" > depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API > diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile > index 1b62639..4bf7d00 100644 > --- a/drivers/media/i2c/Makefile > +++ b/drivers/media/i2c/Makefile > @@ -94,6 +94,7 @@ obj-$(CONFIG_VIDEO_IR_I2C) += ir-kbd-i2c.o > obj-$(CONFIG_VIDEO_ML86V7667) += ml86v7667.o > obj-$(CONFIG_VIDEO_OV2659) += ov2659.o > obj-$(CONFIG_VIDEO_TC358743) += tc358743.o > +obj-$(CONFIG_VIDEO_IMX258) += imx258.o > obj-$(CONFIG_VIDEO_IMX274) += imx274.o > > obj-$(CONFIG_SDR_MAX2175) += max2175.o > diff --git a/drivers/media/i2c/imx258.c b/drivers/media/i2c/imx258.c > new file mode 100644 > index 000..0418edd > --- /dev/null > +++ b/drivers/media/i2c/imx258.c > @@ -0,0 +1,1341 @@ > +// Copyright (C) 2018 Intel Corporation > +// SPDX-License-Identifier: GPL-2.0 > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#define IMX258_REG_VALUE_08BIT 1 > +#define IMX258_REG_VALUE_16BIT 2 > +#define IMX258_REG_VALUE_24BIT 3 Is there any need for these macros? Respective functions could just take the number of bytes as the argument directly. > + > +#define IMX258_REG_MODE_SELECT 0x0100 > +#define IMX258_MODE_STANDBY0x00 > +#define IMX258_MODE_STREAMING 0x01 > + > +/* Chip ID */ > +#define IMX258_REG_CHIP_ID 0x0016 > +#define IMX258_CHIP_ID 0x0258 > + > +/* V_TIMING internal */ > +#define IMX258_REG_VTS 0x0340 > +#define IMX258_VTS_30FPS 0x0c98 > +#define IMX258_VTS_MAX 0x > + > +/*Frame Length Line*/ > +#define IMX258_FLL_MIN 0x08a6 > +#define IMX258_FLL_MAX 0x > +#define IMX258_FLL_STEP1 > +#define IMX258_FLL_DEFAULT 0x0c98 > + > +/* HBLANK control - read only */ > +#define IMX258_PPL_650MHZ 5352 > +#define IMX258_PPL_325MHZ 2676 > + > +/* Exposure control */ > +#define IMX258_REG_EXPOSURE0x0202 > +#define IMX258_EXPOSURE_MIN4 > +#define IMX258_EXPOSURE_STEP 1 > +#define IMX258_EXPOSURE_DEFAULT0x640 > +#define IMX258_EXPOSURE_MAX65535 > + > +/* Analog gain control */ > +#define IMX258_REG_ANALOG_GAIN 0x0204 > +#define IMX258_ANA_GAIN_MIN0 > +#define IMX258_ANA_GAIN_MAX0x1fff > +#define IMX258_ANA_GAIN_STEP 1 > +#define IMX258_ANA_GAIN_DEFAULT0x0 > + > +/* Digital gain control */ > +#define IMX258_REG_GR_DIGITAL_GAIN 0x020e > +#define IMX258_REG_R_DI
[PATCH 1/5] media: i2c: Copy mt9t112 soc_camera sensor driver
Copy the soc_camera based driver in v4l2 sensor driver directory.
This commit just copies the original file without modifying it.
No modification to KConfig and Makefile as soc_camera framework
dependencies need to be removed first in next commit.
Signed-off-by: Jacopo Mondi
---
drivers/media/i2c/mt9t112.c | 1163 +++
1 file changed, 1163 insertions(+)
create mode 100644 drivers/media/i2c/mt9t112.c
diff --git a/drivers/media/i2c/mt9t112.c b/drivers/media/i2c/mt9t112.c
new file mode 100644
index 000..297d22e
--- /dev/null
+++ b/drivers/media/i2c/mt9t112.c
@@ -0,0 +1,1163 @@
+/*
+ * mt9t112 Camera Driver
+ *
+ * Copyright (C) 2009 Renesas Solutions Corp.
+ * Kuninori Morimoto
+ *
+ * Based on ov772x driver, mt9m111 driver,
+ *
+ * Copyright (C) 2008 Kuninori Morimoto
+ * Copyright (C) 2008, Robert Jarzmik
+ * Copyright 2006-7 Jonathan Corbet
+ * Copyright (C) 2008 Magnus Damm
+ * Copyright (C) 2008, Guennadi Liakhovetski
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+#include
+#include
+#include
+#include
+
+/* you can check PLL/clock info */
+/* #define EXT_CLOCK 2400 */
+
+/
+ macro
+/
+/*
+ * frame size
+ */
+#define MAX_WIDTH 2048
+#define MAX_HEIGHT 1536
+
+/*
+ * macro of read/write
+ */
+#define ECHECKER(ret, x) \
+ do {\
+ (ret) = (x);\
+ if ((ret) < 0) \
+ return (ret); \
+ } while (0)
+
+#define mt9t112_reg_write(ret, client, a, b) \
+ ECHECKER(ret, __mt9t112_reg_write(client, a, b))
+#define mt9t112_mcu_write(ret, client, a, b) \
+ ECHECKER(ret, __mt9t112_mcu_write(client, a, b))
+
+#define mt9t112_reg_mask_set(ret, client, a, b, c) \
+ ECHECKER(ret, __mt9t112_reg_mask_set(client, a, b, c))
+#define mt9t112_mcu_mask_set(ret, client, a, b, c) \
+ ECHECKER(ret, __mt9t112_mcu_mask_set(client, a, b, c))
+
+#define mt9t112_reg_read(ret, client, a) \
+ ECHECKER(ret, __mt9t112_reg_read(client, a))
+
+/*
+ * Logical address
+ */
+#define _VAR(id, offset, base) (base | (id & 0x1f) << 10 | (offset & 0x3ff))
+#define VAR(id, offset) _VAR(id, offset, 0x)
+#define VAR8(id, offset) _VAR(id, offset, 0x8000)
+
+/
+ struct
+/
+struct mt9t112_format {
+ u32 code;
+ enum v4l2_colorspace colorspace;
+ u16 fmt;
+ u16 order;
+};
+
+struct mt9t112_priv {
+ struct v4l2_subdev subdev;
+ struct mt9t112_camera_info *info;
+ struct i2c_client *client;
+ struct v4l2_rect frame;
+ struct v4l2_clk *clk;
+ const struct mt9t112_format *format;
+ int num_formats;
+ u32 flags;
+/* for flags */
+#define INIT_DONE (1 << 0)
+#define PCLK_RISING(1 << 1)
+};
+
+/
+ supported format
+/
+
+static const struct mt9t112_format mt9t112_cfmts[] = {
+ {
+ .code = MEDIA_BUS_FMT_UYVY8_2X8,
+ .colorspace = V4L2_COLORSPACE_SRGB,
+ .fmt= 1,
+ .order = 0,
+ }, {
+ .code = MEDIA_BUS_FMT_VYUY8_2X8,
+ .colorspace = V4L2_COLORSPACE_SRGB,
+ .fmt= 1,
+ .order = 1,
+ }, {
+ .code = MEDIA_BUS_FMT_YUYV8_2X8,
+ .colorspace = V4L2_COLORSPACE_SRGB,
+ .fmt= 1,
+ .order = 2,
+ }, {
+ .code = MEDIA_BUS_FMT_YVYU8_2X8,
+ .colorspace = V4L2_COLORSPACE_SRGB,
+ .fmt= 1,
+ .order = 3,
+ }, {
+ .code = MEDIA_BUS_FMT_RGB555_2X8_PADHI_LE,
+ .colorspace = V4L2_COLORSPACE_SRGB,
+ .fmt= 8,
+ .order = 2,
+ }, {
+ .code = MEDIA_BUS_FMT_RGB565_2X8_LE,
+ .colorspace = V4L2_COLORSPACE_SRGB,
+ .fmt= 4,
+ .order = 2,
+ },
+};
+
+/**
[PATCH 5/5] media: MAINTAINERS: Add entry for Aptina MT9T112
Add entry for Aptina/Micron MT9T112 camera sensor. The driver is currently orphaned. Signed-off-by: Jacopo Mondi --- MAINTAINERS | 7 +++ 1 file changed, 7 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 91ed6ad..1d8be25 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -9385,6 +9385,13 @@ S: Maintained F: drivers/media/i2c/mt9t001.c F: include/media/i2c/mt9t001.h +MT9T112 APTINA CAMERA SENSOR +L: linux-media@vger.kernel.org +T: git git://linuxtv.org/media_tree.git +S: Orphan +F: drivers/media/i2c/mt9t112.c +F: include/media/i2c/mt9t112.h + MT9V032 APTINA CAMERA SENSOR M: Laurent Pinchart L: linux-media@vger.kernel.org -- 2.7.4
[PATCH 2/5] media: i2c: mt9t112: Remove soc_camera dependencies
Remove soc_camera framework dependencies from mt9t112 sensor driver.
- Handle clk, gpios and power routines
- Register async subdev
- Remove deprecated g/s_mbus_config operations
- Remove driver flags
- Change driver interface and add kernel doc
- Adjust build system
This commit does not remove the original soc_camera based driver as long
as other platforms depends on soc_camera framework.
As I don't have access to a working camera module, this change has only
been compile tested.
Signed-off-by: Jacopo Mondi
---
drivers/media/i2c/Kconfig | 11
drivers/media/i2c/Makefile | 1 +
drivers/media/i2c/mt9t112.c | 147 +---
include/media/i2c/mt9t112.h | 17 +++--
4 files changed, 89 insertions(+), 87 deletions(-)
diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index d7bba0e..541f0d28 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -788,6 +788,17 @@ config VIDEO_MT9T001
This is a Video4Linux2 sensor-level driver for the Aptina
(Micron) mt0t001 3 Mpixel camera.
+config VIDEO_MT9T112
+ tristate "Aptina MT9T111/MT9T112 support"
+ depends on I2C && VIDEO_V4L2
+ depends on MEDIA_CAMERA_SUPPORT
+ ---help---
+ This is a Video4Linux2 sensor-level driver for the Aptina
+ (Micron) MT9T111 and MT9T112 3 Mpixel camera.
+
+ To compile this driver as a module, choose M here: the
+ module will be called mt9t112.
+
config VIDEO_MT9V011
tristate "Micron mt9v011 sensor support"
depends on I2C && VIDEO_V4L2
diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
index cc30178..ea34aee 100644
--- a/drivers/media/i2c/Makefile
+++ b/drivers/media/i2c/Makefile
@@ -80,6 +80,7 @@ obj-$(CONFIG_VIDEO_MT9M032) += mt9m032.o
obj-$(CONFIG_VIDEO_MT9M111) += mt9m111.o
obj-$(CONFIG_VIDEO_MT9P031) += mt9p031.o
obj-$(CONFIG_VIDEO_MT9T001) += mt9t001.o
+obj-$(CONFIG_VIDEO_MT9T112) += mt9t112.o
obj-$(CONFIG_VIDEO_MT9V011) += mt9v011.o
obj-$(CONFIG_VIDEO_MT9V032) += mt9v032.o
obj-$(CONFIG_VIDEO_SR030PC30) += sr030pc30.o
diff --git a/drivers/media/i2c/mt9t112.c b/drivers/media/i2c/mt9t112.c
index 297d22e..6b77dff 100644
--- a/drivers/media/i2c/mt9t112.c
+++ b/drivers/media/i2c/mt9t112.c
@@ -1,6 +1,9 @@
+// SPDX-License-Identifier: GPL-2.0
/*
* mt9t112 Camera Driver
*
+ * Copyright (C) 2017 Jacopo Mondi
+ *
* Copyright (C) 2009 Renesas Solutions Corp.
* Kuninori Morimoto
*
@@ -11,13 +14,11 @@
* Copyright 2006-7 Jonathan Corbet
* Copyright (C) 2008 Magnus Damm
* Copyright (C) 2008, Guennadi Liakhovetski
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 as
- * published by the Free Software Foundation.
*/
+#include
#include
+#include
#include
#include
#include
@@ -26,10 +27,9 @@
#include
#include
-#include
-#include
#include
#include
+#include
/* you can check PLL/clock info */
/* #define EXT_CLOCK 2400 */
@@ -85,16 +85,14 @@ struct mt9t112_format {
struct mt9t112_priv {
struct v4l2_subdev subdev;
- struct mt9t112_camera_info *info;
+ struct mt9t112_platform_data*info;
struct i2c_client *client;
struct v4l2_rect frame;
- struct v4l2_clk *clk;
+ struct clk *clk;
+ struct gpio_desc*standby_gpio;
const struct mt9t112_format *format;
int num_formats;
- u32 flags;
-/* for flags */
-#define INIT_DONE (1 << 0)
-#define PCLK_RISING(1 << 1)
+ bool init_done;
};
/
@@ -341,12 +339,6 @@ static int mt9t112_clock_info(const struct i2c_client
*client, u32 ext)
}
#endif
-static void mt9t112_frame_check(u32 *width, u32 *height, u32 *left, u32 *top)
-{
- soc_camera_limit_side(left, width, 0, 0, MAX_WIDTH);
- soc_camera_limit_side(top, height, 0, 0, MAX_HEIGHT);
-}
-
static int mt9t112_set_a_frame_size(const struct i2c_client *client,
u16 width,
u16 height)
@@ -764,13 +756,40 @@ static int mt9t112_s_register(struct v4l2_subdev *sd,
}
#endif
+static int mt9t112_power_on(struct mt9t112_priv *priv)
+{
+ int ret;
+
+ ret = clk_prepare_enable(priv->clk);
+ if (ret)
+ return ret;
+
+ if (priv->standby_gpio) {
+ gpiod_set_value(priv->standby_gpio, 0);
+ msleep(100);
+ }
+
+ return 0;
+}
+
+static int mt9t112_power_off(struct mt9t112_priv *priv)
+{
+ clk_disable_unprepare(priv->clk);
+ if (priv->standby_gpio) {
+ gpiod_set_value(priv->standby_gpio, 1);
+ msleep(100)
[PATCH 3/5] media: i2c: mt9t112: Fix code style issues
Fix code style issues reported by checkpatch run with --strict
options. Also fix other non reported style issues manually.
Signed-off-by: Jacopo Mondi
---
drivers/media/i2c/mt9t112.c | 256
1 file changed, 118 insertions(+), 138 deletions(-)
diff --git a/drivers/media/i2c/mt9t112.c b/drivers/media/i2c/mt9t112.c
index 6b77dff..6eaf3c6 100644
--- a/drivers/media/i2c/mt9t112.c
+++ b/drivers/media/i2c/mt9t112.c
@@ -35,8 +35,8 @@
/* #define EXT_CLOCK 2400 */
/
- macro
-/
+ * macro
+ ***/
/*
* frame size
*/
@@ -74,8 +74,8 @@
#define VAR8(id, offset) _VAR(id, offset, 0x8000)
/
- struct
-/
+ * struct
+ ***/
struct mt9t112_format {
u32 code;
enum v4l2_colorspace colorspace;
@@ -96,8 +96,8 @@ struct mt9t112_priv {
};
/
- supported format
-/
+ * supported format
+ ***/
static const struct mt9t112_format mt9t112_cfmts[] = {
{
@@ -134,8 +134,8 @@ static const struct mt9t112_format mt9t112_cfmts[] = {
};
/
- general function
-/
+ * general function
+ ***/
static struct mt9t112_priv *to_mt9t112(const struct i2c_client *client)
{
return container_of(i2c_get_clientdata(client),
@@ -162,15 +162,15 @@ static int __mt9t112_reg_read(const struct i2c_client
*client, u16 command)
msg[1].buf = buf;
/*
-* if return value of this function is < 0,
-* it mean error.
-* else, under 16bit is valid data.
+* If return value of this function is < 0, it means error, else,
+* below 16bit is valid data.
*/
ret = i2c_transfer(client->adapter, msg, 2);
if (ret < 0)
return ret;
memcpy(&ret, buf, 2);
+
return swab16(ret);
}
@@ -193,22 +193,19 @@ static int __mt9t112_reg_write(const struct i2c_client
*client,
msg.buf = buf;
/*
-* i2c_transfer return message length,
-* but this function should return 0 if correct case
+* i2c_transfer return message length, but this function should
+* return 0 if correct case.
*/
ret = i2c_transfer(client->adapter, &msg, 1);
- if (ret >= 0)
- ret = 0;
- return ret;
+ return ret >= 0 ? 0 : ret;
}
static int __mt9t112_reg_mask_set(const struct i2c_client *client,
- u16 command,
- u16 mask,
- u16 set)
+ u16 command, u16 mask, u16 set)
{
int val = __mt9t112_reg_read(client, command);
+
if (val < 0)
return val;
@@ -243,11 +240,10 @@ static int __mt9t112_mcu_write(const struct i2c_client
*client,
}
static int __mt9t112_mcu_mask_set(const struct i2c_client *client,
- u16 command,
- u16 mask,
- u16 set)
+ u16 command, u16 mask, u16 set)
{
int val = __mt9t112_mcu_read(client, command);
+
if (val < 0)
return val;
@@ -262,7 +258,7 @@ static int mt9t112_reset(const struct i2c_client *client)
int ret;
mt9t112_reg_mask_set(ret, client, 0x001a, 0x0001, 0x0001);
- msleep(1);
+ usleep_range(1000, 5000);
mt9t112_reg_mask_set(ret, client, 0x001a, 0x0001, 0x);
return ret;
@@ -301,38 +297,38 @@ static int mt9t112_clock_info(const struct i2c_client
*client, u32 ext)
m = n & 0x00ff;
n = (n >> 8) & 0x003f;
- enable = ((6000 > ext) || (54000 < ext)) ? "X" : "";
+ enable = ((ext < 6000) || (ext > 54000)) ? "X" : "";
dev_dbg(&client->dev, "EXTCLK : %10u K %s\n", ext, enable);
- vco = 2 * m * ext / (n+1);
- enable = ((384000 > vco) || (768000 < vco)) ? "X" : "";
+ vco = 2 * m * ext / (n + 1);
+ enable = ((vco < 384000) || (vco > 768000)) ? "X" : "";
[PATCH 0/5] Renesas CEU: SH7724 ECOVEC + Aptina mt9t112
Hello, now that CEU has been picked up for inclusion in v4.17, we can start moving users of old sh_mobile_ceu_camera driver to use the newly introduced one. Migo-R has been first, now it's SH7724 ECOVEC board turn. ECOVEC has a camera board with two MT9T112 image sensor and one TW9910 video decoder input. This series moves the mt9t112 driver away from soc_camera framework and remove dependencies on it in mach-ecovec board code. As per Migo-R, memory for CEU is reserved using memblocks APIs and declared as DMA-capable in board code, power up/down routines have been removed from board code, and GPIOs lookup table registered for sensor drivers. As in the previous series, still no code has been removed or changed in drivers/media/i2c/soc_camera/ until we do not remove all dependencies on it in all board files. Hans, since you asked me to add frame rate interval support for ov772x I expect to receive the same request for mt9t112. Unfortunately I do not have access to register level documentation, nor can perform any testing as I don't have the camera modules. For the same reason I cannot run any v4l2-compliance test on that driver, but just make sure the ECOVEC boots cleanly with the new board file. I'm in favour of moving the driver to staging if you think that's the case. Series based on media-tree master, and as per Migo-R I would ask SH arch/ changes to go through media tree as SH maintainers are un-responsive. Thanks j Jacopo Mondi (5): media: i2c: Copy mt9t112 soc_camera sensor driver media: i2c: mt9t112: Remove soc_camera dependencies media: i2c: mt9t112: Fix code style issues arch: sh: ecovec: Use new renesas-ceu camera driver media: MAINTAINERS: Add entry for Aptina MT9T112 MAINTAINERS|7 + arch/sh/boards/mach-ecovec24/setup.c | 338 +- arch/sh/kernel/cpu/sh4a/clock-sh7724.c |4 +- drivers/media/i2c/Kconfig | 11 + drivers/media/i2c/Makefile |1 + drivers/media/i2c/mt9t112.c| 1136 include/media/i2c/mt9t112.h| 17 +- 7 files changed, 1333 insertions(+), 181 deletions(-) create mode 100644 drivers/media/i2c/mt9t112.c -- 2.7.4
[PATCH 4/5] arch: sh: ecovec: Use new renesas-ceu camera driver
SH4 7724 Ecovec platform uses sh_mobile_ceu camera driver, which is now
being replaced by a proper V4L2 camera driver named 'renesas-ceu'.
Get rid of soc_camera defined components used to register sensor drivers
and of platform specific enable/disable routines.
Register GPIOs for sensor drivers and declare memory reserved with
memblock APIs as dma capable to be used for CEU buffers.
While at there re-order include directives to respect alphabetical
ordering.
Signed-off-by: Jacopo Mondi
---
arch/sh/boards/mach-ecovec24/setup.c | 338 -
arch/sh/kernel/cpu/sh4a/clock-sh7724.c | 4 +-
2 files changed, 171 insertions(+), 171 deletions(-)
diff --git a/arch/sh/boards/mach-ecovec24/setup.c
b/arch/sh/boards/mach-ecovec24/setup.c
index 6f929ab..adc61d1 100644
--- a/arch/sh/boards/mach-ecovec24/setup.c
+++ b/arch/sh/boards/mach-ecovec24/setup.c
@@ -7,44 +7,47 @@
* License. See the file "COPYING" in the main directory of this archive
* for more details.
*/
-
-#include
+#include
+#include
+#include
+#include
+#include
#include
-#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
#include
#include
#include
-#include
#include
-#include
-#include
-#include
+#include
+#include
+#include
+#include
#include
#include
-#include
-#include
-#include
-#include
-#include
-#include
-#include
-#include
-#include
-#include
#include
#include
+#include
+#include
+#include
+#include
+#include
#include
-#include
+
+#include
+#include
+#include
+
#include
#include
-#include
-#include
-#include
-#include
-#include
-#include
-#include
-#include
+
+#include
/*
* Address InterfaceBusWidth
@@ -81,6 +84,10 @@
* amixer set 'Out Mixer Right DAC Right' on
*/
+#define CEU_BUFFER_MEMORY_SIZE (4 << 20)
+static phys_addr_t ceu0_dma_membase;
+static phys_addr_t ceu1_dma_membase;
+
/* Heartbeat */
static unsigned char led_pos[] = { 0, 1, 2, 3 };
@@ -382,8 +389,24 @@ static struct platform_device gpio_backlight_device = {
};
/* CEU0 */
-static struct sh_mobile_ceu_info sh_mobile_ceu0_info = {
- .flags = SH_CEU_FLAG_USE_8BIT_BUS,
+static struct ceu_platform_data ceu0_pdata = {
+ .num_subdevs= 2,
+ .subdevs = {
+ { /* [0] = mt9t112 */
+ .flags = 0,
+ .bus_width = 8,
+ .bus_shift = 0,
+ .i2c_adapter_id = 0,
+ .i2c_address= 0x3c,
+ },
+ { /* [1] = tw9910 */
+ .flags = 0,
+ .bus_width = 8,
+ .bus_shift = 0,
+ .i2c_adapter_id = 0,
+ .i2c_address= 0x45,
+ },
+ },
};
static struct resource ceu0_resources[] = {
@@ -397,24 +420,30 @@ static struct resource ceu0_resources[] = {
.start = evt2irq(0x880),
.flags = IORESOURCE_IRQ,
},
- [2] = {
- /* place holder for contiguous memory */
- },
};
static struct platform_device ceu0_device = {
- .name = "sh_mobile_ceu",
- .id = 0, /* "ceu0" clock */
+ .name = "renesas-ceu",
+ .id = 0, /* ceu.0 */
.num_resources = ARRAY_SIZE(ceu0_resources),
.resource = ceu0_resources,
.dev= {
- .platform_data = &sh_mobile_ceu0_info,
+ .platform_data = &ceu0_pdata,
},
};
/* CEU1 */
-static struct sh_mobile_ceu_info sh_mobile_ceu1_info = {
- .flags = SH_CEU_FLAG_USE_8BIT_BUS,
+static struct ceu_platform_data ceu1_pdata = {
+ .num_subdevs= 1,
+ .subdevs = {
+ { /* [0] = mt9t112 */
+ .flags = 0,
+ .bus_width = 8,
+ .bus_shift = 0,
+ .i2c_adapter_id = 1,
+ .i2c_address= 0x3c,
+ },
+ },
};
static struct resource ceu1_resources[] = {
@@ -428,26 +457,71 @@ static struct resource ceu1_resources[] = {
.start = evt2irq(0x9e0),
.flags = IORESOURCE_IRQ,
},
- [2] = {
- /* place holder for contiguous memory */
- },
};
static struct platform_device ceu1_device = {
- .name = "sh_mobile_ceu",
- .id = 1, /* "ceu1" clock */
+ .name = "renesas-ceu",
+ .id = 1, /* ceu.1 */
.num_resources = ARRAY_SIZE(ceu1_resources),
.resource = ceu1_resources,
.dev= {
- .platform_data = &sh_mobile_ceu1_info,
+ .platform_data = &ceu1_pdata,
+ },
+};
+
+/* Power up/down GPIOs for camera de
Re: [v5 2/2] media: dt-bindings: Add bindings for Dongwoon DW9807 voice coil
On Wed, Feb 28, 2018 at 03:31:26PM +0200, Sakari Ailus wrote: > Hi Rob, > > Thanks for the review. > > On Tue, Feb 27, 2018 at 04:10:31PM -0600, Rob Herring wrote: > > On Fri, Feb 23, 2018 at 10:13 AM, Andy Yeh wrote: > > > From: Alan Chiang > > > > > > Dongwoon DW9807 is a voice coil lens driver. > > > > > > Also add a vendor prefix for Dongwoon for one did not exist previously. > > > > Where's that? > > Added by aece98a912d92444ea9da03b04269407d1308f1f . So that line isn't > relevant indeed and should be removed. > > > > > > > > > Signed-off-by: Andy Yeh > > > --- > > > Documentation/devicetree/bindings/media/i2c/dongwoon,dw9807.txt | 9 > > > + > > > > DACs generally go in bindings/iio/dac/ > > We have quite a few lens voice coil drivers under bindings/media/i2c now. I > don't really object to putting this one to bindings/iio/dac but then the > rest should be moved as well. > > The camera LED flash drivers are under bindings/leds so this would actually > be analoguous to that. The lens voice coil drivers are perhaps still a bit > more bound to the domain (camera) than the LED flash drivers. The h/w is bound to that function or just the s/w? > I can send a patch if you think the existing bindings should be moved; let > me know. I'm okay if they are separate as long as we're not going to see the same device show up in both places. However, "i2c" is not the best directory choice. It should be by function, so we can find common properties. Rob
[PATCH 1/2] media: imx-media-csi: Fix inconsistent IS_ERR and PTR_ERR
From: Gustavo A. R. Silva
Fix inconsistent IS_ERR and PTR_ERR in imx_csi_probe.
The proper pointer to be passed as argument is pinctrl
instead of priv->vdev.
This issue was detected with the help of Coccinelle.
Fixes: 52e17089d185 ("media: imx: Don't initialize vars that won't be used")
Signed-off-by: Gustavo A. R. Silva
Signed-off-by: Fabio Estevam
Acked-by: Philipp Zabel
---
drivers/staging/media/imx/imx-media-csi.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/media/imx/imx-media-csi.c
b/drivers/staging/media/imx/imx-media-csi.c
index 5a195f8..4f290a0 100644
--- a/drivers/staging/media/imx/imx-media-csi.c
+++ b/drivers/staging/media/imx/imx-media-csi.c
@@ -1798,7 +1798,7 @@ static int imx_csi_probe(struct platform_device *pdev)
priv->dev->of_node = pdata->of_node;
pinctrl = devm_pinctrl_get_select_default(priv->dev);
if (IS_ERR(pinctrl)) {
- ret = PTR_ERR(priv->vdev);
+ ret = PTR_ERR(pinctrl);
goto free;
}
--
2.7.4
[PATCH 2/2] media: imx-media-csi: Do not propagate the error when pinctrl is not found
From: Fabio Estevam
Since commit 52e17089d185 ("media: imx: Don't initialize vars that
won't be used") imx_csi_probe() fails to probe after propagating the
devm_pinctrl_get_select_default() error.
devm_pinctrl_get_select_default() may return -ENODEV when the CSI pinctrl
entry is not found, so better not to propagate the error in the -ENODEV
case to avoid a regression.
Suggested-by: Philipp Zabel
Signed-off-by: Fabio Estevam
---
drivers/staging/media/imx/imx-media-csi.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/media/imx/imx-media-csi.c
b/drivers/staging/media/imx/imx-media-csi.c
index 4f290a0..5af66f6 100644
--- a/drivers/staging/media/imx/imx-media-csi.c
+++ b/drivers/staging/media/imx/imx-media-csi.c
@@ -1799,7 +1799,10 @@ static int imx_csi_probe(struct platform_device *pdev)
pinctrl = devm_pinctrl_get_select_default(priv->dev);
if (IS_ERR(pinctrl)) {
ret = PTR_ERR(pinctrl);
- goto free;
+ dev_dbg(priv->dev,
+ "devm_pinctrl_get_select_default() failed: %d", ret);
+ if (ret != -ENODEV)
+ goto free;
}
ret = v4l2_async_register_subdev(&priv->sd);
--
2.7.4
[PATCH 1/8] media: em28xx: don't use coherent buffer for DMA transfers
While coherent memory is cheap on x86, it may cause performance
impacts on other archs. As we don't have any good reason to
use it, let's change the logic by allocating memory via kmalloc()
and letting the USB core to do the DMA mapping and memory free
for us.
While here, also fixes an issue that it was not de-allocating
memories if something gets wrong during memory block
allocation.
Signed-off-by: Mauro Carvalho Chehab
---
drivers/media/usb/em28xx/em28xx-core.c | 51 +++---
drivers/media/usb/em28xx/em28xx.h | 2 +-
2 files changed, 24 insertions(+), 29 deletions(-)
diff --git a/drivers/media/usb/em28xx/em28xx-core.c
b/drivers/media/usb/em28xx/em28xx-core.c
index 1d0d8cc06103..932ab93a05a6 100644
--- a/drivers/media/usb/em28xx/em28xx-core.c
+++ b/drivers/media/usb/em28xx/em28xx-core.c
@@ -800,7 +800,6 @@ void em28xx_uninit_usb_xfer(struct em28xx *dev, enum
em28xx_mode mode)
{
struct urb *urb;
struct em28xx_usb_bufs *usb_bufs;
- struct usb_device *udev = interface_to_usbdev(dev->intf);
int i;
em28xx_isocdbg("em28xx: called em28xx_uninit_usb_xfer in mode %d\n",
@@ -819,23 +818,16 @@ void em28xx_uninit_usb_xfer(struct em28xx *dev, enum
em28xx_mode mode)
else
usb_unlink_urb(urb);
- if (usb_bufs->transfer_buffer[i]) {
- usb_free_coherent(udev,
- urb->transfer_buffer_length,
- usb_bufs->transfer_buffer[i],
- urb->transfer_dma);
- }
usb_free_urb(urb);
usb_bufs->urb[i] = NULL;
}
- usb_bufs->transfer_buffer[i] = NULL;
}
kfree(usb_bufs->urb);
- kfree(usb_bufs->transfer_buffer);
+ kfree(usb_bufs->buf);
usb_bufs->urb = NULL;
- usb_bufs->transfer_buffer = NULL;
+ usb_bufs->buf = NULL;
usb_bufs->num_bufs = 0;
em28xx_capture_start(dev, 0);
@@ -912,14 +904,13 @@ int em28xx_alloc_urbs(struct em28xx *dev, enum
em28xx_mode mode, int xfer_bulk,
usb_bufs->num_bufs = num_bufs;
- usb_bufs->urb = kzalloc(sizeof(void *)*num_bufs, GFP_KERNEL);
+ usb_bufs->urb = kcalloc(sizeof(void *), num_bufs, GFP_KERNEL);
if (!usb_bufs->urb)
return -ENOMEM;
- usb_bufs->transfer_buffer = kzalloc(sizeof(void *)*num_bufs,
-GFP_KERNEL);
- if (!usb_bufs->transfer_buffer) {
- kfree(usb_bufs->urb);
+ usb_bufs->buf = kcalloc(sizeof(void *), num_bufs, GFP_KERNEL);
+ if (!usb_bufs->buf) {
+ kfree(usb_bufs->buf);
return -ENOMEM;
}
@@ -942,37 +933,41 @@ int em28xx_alloc_urbs(struct em28xx *dev, enum
em28xx_mode mode, int xfer_bulk,
}
usb_bufs->urb[i] = urb;
- usb_bufs->transfer_buffer[i] = usb_alloc_coherent(udev,
- sb_size, GFP_KERNEL, &urb->transfer_dma);
- if (!usb_bufs->transfer_buffer[i]) {
+ usb_bufs->buf[i] = kzalloc(sb_size, GFP_KERNEL);
+ if (!usb_bufs->buf[i]) {
dev_err(&dev->intf->dev,
"unable to allocate %i bytes for transfer
buffer %i%s\n",
sb_size, i,
in_interrupt() ? " while in int" : "");
+
em28xx_uninit_usb_xfer(dev, mode);
+
+ for (i--; i >= 0; i--)
+ kfree(usb_bufs->buf[i]);
+
+ kfree(usb_bufs->buf);
+ usb_bufs->buf = NULL;
+
return -ENOMEM;
}
- memset(usb_bufs->transfer_buffer[i], 0, sb_size);
+
+ urb->transfer_flags = URB_FREE_BUFFER;
if (xfer_bulk) { /* bulk */
pipe = usb_rcvbulkpipe(udev,
mode == EM28XX_ANALOG_MODE ?
dev->analog_ep_bulk :
dev->dvb_ep_bulk);
- usb_fill_bulk_urb(urb, udev, pipe,
- usb_bufs->transfer_buffer[i], sb_size,
- em28xx_irq_callback, dev);
- urb->transfer_flags = URB_NO_TRANSFER_DMA_MAP;
+ usb_fill_bulk_urb(urb, udev, pipe, usb_bufs->buf[i],
+ sb_size, em28xx_irq_callback, dev);
} else { /* isoc */
pipe = usb_rcvisocpipe(udev,
mode == EM28XX_ANALOG_MODE ?
[PATCH 3/8] media: em28xx: stop rewriting device's struct
Writing at the device's struct is evil, as two em28xx devices
may be using it. So, stop abusing it, storing the values
inside struct em28xx_dev.
Signed-off-by: Mauro Carvalho Chehab
---
drivers/media/usb/em28xx/em28xx-cards.c | 12 +++-
drivers/media/usb/em28xx/em28xx-core.c | 4 ++--
drivers/media/usb/em28xx/em28xx-video.c | 24
drivers/media/usb/em28xx/em28xx.h | 2 ++
4 files changed, 23 insertions(+), 19 deletions(-)
diff --git a/drivers/media/usb/em28xx/em28xx-cards.c
b/drivers/media/usb/em28xx/em28xx-cards.c
index cbd7a43bd559..91c2198c8a2c 100644
--- a/drivers/media/usb/em28xx/em28xx-cards.c
+++ b/drivers/media/usb/em28xx/em28xx-cards.c
@@ -2696,6 +2696,8 @@ static inline void em28xx_set_xclk_i2c_speed(struct
em28xx *dev)
static inline void em28xx_set_model(struct em28xx *dev)
{
dev->board = em28xx_boards[dev->model];
+ dev->has_msp34xx = dev->board.has_msp34xx;
+ dev->is_webcam = dev->board.is_webcam;
em28xx_set_xclk_i2c_speed(dev);
@@ -2855,7 +2857,7 @@ static int em28xx_hint_board(struct em28xx *dev)
{
int i;
- if (dev->board.is_webcam) {
+ if (dev->is_webcam) {
if (dev->em28xx_sensor == EM28XX_MT9V011) {
dev->model = EM2820_BOARD_SILVERCREST_WEBCAM;
} else if (dev->em28xx_sensor == EM28XX_MT9M001 ||
@@ -2947,11 +2949,11 @@ static void em28xx_card_setup(struct em28xx *dev)
* If the device can be a webcam, seek for a sensor.
* If sensor is not found, then it isn't a webcam.
*/
- if (dev->board.is_webcam) {
+ if (dev->is_webcam) {
em28xx_detect_sensor(dev);
if (dev->em28xx_sensor == EM28XX_NOSENSOR)
/* NOTE: error/unknown sensor/no sensor */
- dev->board.is_webcam = 0;
+ dev->is_webcam = 0;
}
switch (dev->model) {
@@ -3013,7 +3015,7 @@ static void em28xx_card_setup(struct em28xx *dev)
if (tv.audio_processor == TVEEPROM_AUDPROC_MSP) {
dev->i2s_speed = 2048000;
- dev->board.has_msp34xx = 1;
+ dev->has_msp34xx = 1;
}
break;
}
@@ -3679,7 +3681,7 @@ static int em28xx_usb_probe(struct usb_interface
*interface,
}
if (usb_xfer_mode < 0) {
- if (dev->board.is_webcam)
+ if (dev->is_webcam)
try_bulk = 1;
else
try_bulk = 0;
diff --git a/drivers/media/usb/em28xx/em28xx-core.c
b/drivers/media/usb/em28xx/em28xx-core.c
index 932ab93a05a6..92c99b0320b3 100644
--- a/drivers/media/usb/em28xx/em28xx-core.c
+++ b/drivers/media/usb/em28xx/em28xx-core.c
@@ -378,7 +378,7 @@ static int em28xx_set_audio_source(struct em28xx *dev)
return ret;
}
- if (dev->board.has_msp34xx)
+ if (dev->has_msp34xx)
input = EM28XX_AUDIO_SRC_TUNER;
else {
switch (dev->ctl_ainput) {
@@ -651,7 +651,7 @@ int em28xx_capture_start(struct em28xx *dev, int start)
return rc;
if (start) {
- if (dev->board.is_webcam)
+ if (dev->is_webcam)
rc = em28xx_write_reg(dev, 0x13, 0x0c);
/* Enable video capture */
diff --git a/drivers/media/usb/em28xx/em28xx-video.c
b/drivers/media/usb/em28xx/em28xx-video.c
index a2ba2d905952..407850d9cae0 100644
--- a/drivers/media/usb/em28xx/em28xx-video.c
+++ b/drivers/media/usb/em28xx/em28xx-video.c
@@ -148,7 +148,7 @@ static inline unsigned int norm_maxw(struct em28xx *dev)
{
struct em28xx_v4l2 *v4l2 = dev->v4l2;
- if (dev->board.is_webcam)
+ if (dev->is_webcam)
return v4l2->sensor_xres;
if (dev->board.max_range_640_480)
@@ -161,7 +161,7 @@ static inline unsigned int norm_maxh(struct em28xx *dev)
{
struct em28xx_v4l2 *v4l2 = dev->v4l2;
- if (dev->board.is_webcam)
+ if (dev->is_webcam)
return v4l2->sensor_yres;
if (dev->board.max_range_640_480)
@@ -176,7 +176,7 @@ static int em28xx_vbi_supported(struct em28xx *dev)
if (disable_vbi == 1)
return 0;
- if (dev->board.is_webcam)
+ if (dev->is_webcam)
return 0;
/* FIXME: check subdevices for VBI support */
@@ -976,7 +976,7 @@ static void em28xx_v4l2_create_entities(struct em28xx *dev)
}
/* Webcams don't have input connectors */
- if (dev->board.is_webcam)
+ if (dev->is_webcam)
return;
/* Create entities for each input connector */
@@ -1277,7 +1277,7 @@ static void video_mux(struct em28xx *dev, int index)
v4l2_device_call_all(v4l2_dev, 0, video, s_routing,
INPUT
[PATCH 6/8] media: em28xx: split up em28xx_dvb_init to reduce stack size
From: Arnd Bergmann
With CONFIG_KASAN, the init function uses a large amount of kernel stack:
drivers/media/usb/em28xx/em28xx-dvb.c: In function 'em28xx_dvb_init.part.4':
drivers/media/usb/em28xx/em28xx-dvb.c:2061:1: error: the frame size of 3232
bytes is larger than 2048 bytes [-Werror=frame-larger-than=]
Using gcc-7 with -fsanitize-address-use-after-scope makes this even worse:
drivers/media/usb/em28xx/em28xx-dvb.c: In function 'em28xx_dvb_init':
drivers/media/usb/em28xx/em28xx-dvb.c:2069:1: error: the frame size of 4280
bytes is larger than 3072 bytes [-Werror=frame-larger-than=]
By splitting out each part of the switch/case statement that has its own local
variables into a separate function, no single one of them uses more than 500
bytes,
and with a noinline_for_stack annotation we can ensure that they are not merged
back together.
Signed-off-by: Arnd Bergmann
Signed-off-by: Mauro Carvalho Chehab
---
drivers/media/usb/em28xx/em28xx-dvb.c | 947 ++
1 file changed, 508 insertions(+), 439 deletions(-)
diff --git a/drivers/media/usb/em28xx/em28xx-dvb.c
b/drivers/media/usb/em28xx/em28xx-dvb.c
index 8a81c94a8a27..28c4c7d8dbd8 100644
--- a/drivers/media/usb/em28xx/em28xx-dvb.c
+++ b/drivers/media/usb/em28xx/em28xx-dvb.c
@@ -934,7 +934,7 @@ static struct lgdt3306a_config
hauppauge_01595_lgdt3306a_config = {
/* -- */
-static int em28xx_attach_xc3028(u8 addr, struct em28xx *dev)
+static noinline_for_stack int em28xx_attach_xc3028(u8 addr, struct em28xx *dev)
{
struct dvb_frontend *fe;
struct xc2028_config cfg;
@@ -1126,6 +1126,492 @@ static void em28xx_unregister_dvb(struct em28xx_dvb
*dvb)
dvb_unregister_adapter(&dvb->adapter);
}
+static noinline_for_stack int em28174_dvb_init_pctv_460e(struct em28xx *dev)
+{
+ struct em28xx_dvb *dvb = dev->dvb;
+ struct i2c_client *client;
+ struct i2c_board_info board_info;
+ struct tda10071_platform_data tda10071_pdata = {};
+ struct a8293_platform_data a8293_pdata = {};
+ int result;
+
+ /* attach demod + tuner combo */
+ tda10071_pdata.clk = 40444000, /* 40.444 MHz */
+ tda10071_pdata.i2c_wr_max = 64,
+ tda10071_pdata.ts_mode = TDA10071_TS_SERIAL,
+ tda10071_pdata.pll_multiplier = 20,
+ tda10071_pdata.tuner_i2c_addr = 0x14,
+ memset(&board_info, 0, sizeof(board_info));
+ strlcpy(board_info.type, "tda10071_cx24118", I2C_NAME_SIZE);
+ board_info.addr = 0x55;
+ board_info.platform_data = &tda10071_pdata;
+ request_module("tda10071");
+ client = i2c_new_device(&dev->i2c_adap[dev->def_i2c_bus], &board_info);
+ if (client == NULL || client->dev.driver == NULL) {
+ result = -ENODEV;
+ goto out_free;
+ }
+ if (!try_module_get(client->dev.driver->owner)) {
+ i2c_unregister_device(client);
+ result = -ENODEV;
+ goto out_free;
+ }
+ dvb->fe[0] = tda10071_pdata.get_dvb_frontend(client);
+ dvb->i2c_client_demod = client;
+
+ /* attach SEC */
+ a8293_pdata.dvb_frontend = dvb->fe[0];
+ memset(&board_info, 0, sizeof(board_info));
+ strlcpy(board_info.type, "a8293", I2C_NAME_SIZE);
+ board_info.addr = 0x08;
+ board_info.platform_data = &a8293_pdata;
+ request_module("a8293");
+ client = i2c_new_device(&dev->i2c_adap[dev->def_i2c_bus], &board_info);
+ if (client == NULL || client->dev.driver == NULL) {
+ module_put(dvb->i2c_client_demod->dev.driver->owner);
+ i2c_unregister_device(dvb->i2c_client_demod);
+ result = -ENODEV;
+ goto out_free;
+ }
+ if (!try_module_get(client->dev.driver->owner)) {
+ i2c_unregister_device(client);
+ module_put(dvb->i2c_client_demod->dev.driver->owner);
+ i2c_unregister_device(dvb->i2c_client_demod);
+ result = -ENODEV;
+ goto out_free;
+ }
+ dvb->i2c_client_sec = client;
+ result = 0;
+out_free:
+ return result;
+}
+
+static noinline_for_stack int em28178_dvb_init_pctv_461e(struct em28xx *dev)
+{
+ struct em28xx_dvb *dvb = dev->dvb;
+ struct i2c_client *client;
+ struct i2c_adapter *i2c_adapter;
+ struct i2c_board_info board_info;
+ struct m88ds3103_platform_data m88ds3103_pdata = {};
+ struct ts2020_config ts2020_config = {};
+ struct a8293_platform_data a8293_pdata = {};
+ int result;
+
+ /* attach demod */
+ m88ds3103_pdata.clk = 2700;
+ m88ds3103_pdata.i2c_wr_max = 33;
+ m88ds3103_pdata.ts_mode = M88DS3103_TS_PARALLEL;
+ m88ds3103_pdata.ts_clk = 16000;
+ m88ds3103_pdata.ts_clk_pol = 1;
+ m88ds3103_pdata.agc = 0x99;
+ memset(&board_info, 0, sizeof(board_info));
+ strlcpy(board_info.type,
[PATCH 8/8] media: em28xx-dvb: simplify DVB module probing logic
The module probing logic there is a way more complex than
it should be, and requires some special magic to avoid
stack overflows when KASAN is enabled.
Solve it by creating ancillary functions to setup the
platform data and request module.
Now, the probing functions are cleaner and easier to understand.
As a side effect, the size of the module was reduced by
about 9.7% on x86_64:
Before this patch:
textdata bss dec hex filename
51090 14192 96 65378ff62 drivers/media/usb/em28xx/em28xx-dvb.o
After this patch:
textdata bss dec hex filename
44743 14192 96 59031e697 drivers/media/usb/em28xx/em28xx-dvb.o
Tested with a PCTV 461e device.
Signed-off-by: Mauro Carvalho Chehab
---
drivers/media/usb/em28xx/em28xx-dvb.c | 528 +-
1 file changed, 138 insertions(+), 390 deletions(-)
diff --git a/drivers/media/usb/em28xx/em28xx-dvb.c
b/drivers/media/usb/em28xx/em28xx-dvb.c
index 28c4c7d8dbd8..435c2dc31e90 100644
--- a/drivers/media/usb/em28xx/em28xx-dvb.c
+++ b/drivers/media/usb/em28xx/em28xx-dvb.c
@@ -1126,76 +1126,48 @@ static void em28xx_unregister_dvb(struct em28xx_dvb
*dvb)
dvb_unregister_adapter(&dvb->adapter);
}
-static noinline_for_stack int em28174_dvb_init_pctv_460e(struct em28xx *dev)
+static int em28174_dvb_init_pctv_460e(struct em28xx *dev)
{
struct em28xx_dvb *dvb = dev->dvb;
- struct i2c_client *client;
- struct i2c_board_info board_info;
struct tda10071_platform_data tda10071_pdata = {};
struct a8293_platform_data a8293_pdata = {};
- int result;
/* attach demod + tuner combo */
- tda10071_pdata.clk = 40444000, /* 40.444 MHz */
- tda10071_pdata.i2c_wr_max = 64,
- tda10071_pdata.ts_mode = TDA10071_TS_SERIAL,
- tda10071_pdata.pll_multiplier = 20,
- tda10071_pdata.tuner_i2c_addr = 0x14,
- memset(&board_info, 0, sizeof(board_info));
- strlcpy(board_info.type, "tda10071_cx24118", I2C_NAME_SIZE);
- board_info.addr = 0x55;
- board_info.platform_data = &tda10071_pdata;
- request_module("tda10071");
- client = i2c_new_device(&dev->i2c_adap[dev->def_i2c_bus], &board_info);
- if (client == NULL || client->dev.driver == NULL) {
- result = -ENODEV;
- goto out_free;
- }
- if (!try_module_get(client->dev.driver->owner)) {
- i2c_unregister_device(client);
- result = -ENODEV;
- goto out_free;
- }
- dvb->fe[0] = tda10071_pdata.get_dvb_frontend(client);
- dvb->i2c_client_demod = client;
+ tda10071_pdata.clk = 40444000; /* 40.444 MHz */
+ tda10071_pdata.i2c_wr_max = 64;
+ tda10071_pdata.ts_mode = TDA10071_TS_SERIAL;
+ tda10071_pdata.pll_multiplier = 20;
+ tda10071_pdata.tuner_i2c_addr = 0x14;
+
+ dvb->i2c_client_demod = dvb_module_probe("tda10071", "tda10071_cx24118",
+
&dev->i2c_adap[dev->def_i2c_bus],
+0x55, &tda10071_pdata);
+ if (!dvb->i2c_client_demod)
+ return -ENODEV;
+
+ dvb->fe[0] = tda10071_pdata.get_dvb_frontend(dvb->i2c_client_demod);
/* attach SEC */
a8293_pdata.dvb_frontend = dvb->fe[0];
- memset(&board_info, 0, sizeof(board_info));
- strlcpy(board_info.type, "a8293", I2C_NAME_SIZE);
- board_info.addr = 0x08;
- board_info.platform_data = &a8293_pdata;
- request_module("a8293");
- client = i2c_new_device(&dev->i2c_adap[dev->def_i2c_bus], &board_info);
- if (client == NULL || client->dev.driver == NULL) {
- module_put(dvb->i2c_client_demod->dev.driver->owner);
- i2c_unregister_device(dvb->i2c_client_demod);
- result = -ENODEV;
- goto out_free;
+
+ dvb->i2c_client_sec = dvb_module_probe("a8293", NULL,
+ &dev->i2c_adap[dev->def_i2c_bus],
+ 0x08, &a8293_pdata);
+ if (!dvb->i2c_client_sec) {
+ dvb_module_release(dvb->i2c_client_demod);
+ return -ENODEV;
}
- if (!try_module_get(client->dev.driver->owner)) {
- i2c_unregister_device(client);
- module_put(dvb->i2c_client_demod->dev.driver->owner);
- i2c_unregister_device(dvb->i2c_client_demod);
- result = -ENODEV;
- goto out_free;
- }
- dvb->i2c_client_sec = client;
- result = 0;
-out_free:
- return result;
+
+ return 0;
}
-static noinline_for_stack int em28178_dvb_init_pctv_461e(struct em28xx *dev)
+static int em28178_dvb_init_pctv_461e(struct em28xx *dev)
{
struct em28xx_dvb *dvb = dev->dvb;
- struct i2c_client *client;
struct i2c_adapter *i2c_adapter;
- struct i2c_board_info board_info;
[PATCH 5/8] media: em28xx: adjust I2C timeout according with I2C speed
If the I2C speed is too slow, it should wait more for an
answer.
While here, change disconnected type from char to unsigned
int, just like all other bitmask fields there at em28xx
struct.
Signed-off-by: Mauro Carvalho Chehab
---
drivers/media/usb/em28xx/em28xx-cards.c | 2 ++
drivers/media/usb/em28xx/em28xx-i2c.c | 36 ++---
drivers/media/usb/em28xx/em28xx.h | 19 ++---
3 files changed, 37 insertions(+), 20 deletions(-)
diff --git a/drivers/media/usb/em28xx/em28xx-cards.c
b/drivers/media/usb/em28xx/em28xx-cards.c
index 02af067d7451..03bd3ee19419 100644
--- a/drivers/media/usb/em28xx/em28xx-cards.c
+++ b/drivers/media/usb/em28xx/em28xx-cards.c
@@ -2688,6 +2688,8 @@ static inline void em28xx_set_xclk_i2c_speed(struct
em28xx *dev)
i2c_speed = EM28XX_I2C_CLK_WAIT_ENABLE |
EM28XX_I2C_FREQ_100_KHZ;
+ dev->i2c_speed = i2c_speed & 0x03;
+
if (!dev->board.is_em2800)
em28xx_write_reg(dev, EM28XX_R06_I2C_CLK, i2c_speed);
msleep(50);
diff --git a/drivers/media/usb/em28xx/em28xx-i2c.c
b/drivers/media/usb/em28xx/em28xx-i2c.c
index 9bf49d666e5a..f9aaee3f3d91 100644
--- a/drivers/media/usb/em28xx/em28xx-i2c.c
+++ b/drivers/media/usb/em28xx/em28xx-i2c.c
@@ -51,13 +51,43 @@ MODULE_PARM_DESC(i2c_debug, "i2c debug message level (1:
normal debug, 2: show I
} while (0)
+/*
+ * Time in msecs to wait for i2c xfers to finish.
+ * 35ms is the maximum time a SMBUS device could wait when
+ * clock stretching is used. As the transfer itself will take
+ * some time to happen, set it to 35 ms.
+ *
+ * Ok, I2C doesn't specify any limit. So, eventually, we may need
+ * to increase this timeout.
+ */
+#define EM28XX_I2C_XFER_TIMEOUT 35 /* ms */
+
+static int em28xx_i2c_timeout(struct em28xx *dev)
+{
+ int time = EM28XX_I2C_XFER_TIMEOUT;
+
+ switch (dev->i2c_speed & 0x03) {
+ case EM28XX_I2C_FREQ_25_KHZ:
+ return time += 4; /* Assume 4 ms for transfers */
+ break;
+ case EM28XX_I2C_FREQ_100_KHZ:
+ case EM28XX_I2C_FREQ_400_KHZ:
+ return time += 1; /* Assume 1 ms for transfers */
+ break;
+ default: /* EM28XX_I2C_FREQ_1_5_MHZ */
+ break;
+ }
+
+ return msecs_to_jiffies(time);
+}
+
/*
* em2800_i2c_send_bytes()
* send up to 4 bytes to the em2800 i2c device
*/
static int em2800_i2c_send_bytes(struct em28xx *dev, u8 addr, u8 *buf, u16 len)
{
- unsigned long timeout = jiffies +
msecs_to_jiffies(EM28XX_I2C_XFER_TIMEOUT);
+ unsigned long timeout = jiffies + em28xx_i2c_timeout(dev);
int ret;
u8 b2[6];
@@ -110,7 +140,7 @@ static int em2800_i2c_send_bytes(struct em28xx *dev, u8
addr, u8 *buf, u16 len)
*/
static int em2800_i2c_recv_bytes(struct em28xx *dev, u8 addr, u8 *buf, u16 len)
{
- unsigned long timeout = jiffies +
msecs_to_jiffies(EM28XX_I2C_XFER_TIMEOUT);
+ unsigned long timeout = jiffies + em28xx_i2c_timeout(dev);
u8 buf2[4];
int ret;
int i;
@@ -186,7 +216,7 @@ static int em2800_i2c_check_for_device(struct em28xx *dev,
u8 addr)
static int em28xx_i2c_send_bytes(struct em28xx *dev, u16 addr, u8 *buf,
u16 len, int stop)
{
- unsigned long timeout = jiffies +
msecs_to_jiffies(EM28XX_I2C_XFER_TIMEOUT);
+ unsigned long timeout = jiffies + em28xx_i2c_timeout(dev);
int ret;
if (len < 1 || len > 64)
diff --git a/drivers/media/usb/em28xx/em28xx.h
b/drivers/media/usb/em28xx/em28xx.h
index b23f323b5c99..220e7a7a6124 100644
--- a/drivers/media/usb/em28xx/em28xx.h
+++ b/drivers/media/usb/em28xx/em28xx.h
@@ -195,22 +195,6 @@
#define EM28XX_INTERLACED_DEFAULT 1
-/*
- * Time in msecs to wait for i2c xfers to finish.
- * 35ms is the maximum time a SMBUS device could wait when
- * clock stretching is used. As the transfer itself will take
- * some time to happen, set it to 35 ms.
- *
- * Ok, I2C doesn't specify any limit. So, eventually, we may need
- * to increase this timeout.
- *
- * FIXME: this assumes that an I2C message is not longer than 1ms.
- * This is actually dependent on the I2C bus speed, although most
- * devices use a 100kHz clock. So, this assumtion is true most of
- * the time.
- */
-#define EM28XX_I2C_XFER_TIMEOUT36
-
/* time in msecs to wait for AC97 xfers to finish */
#define EM28XX_AC97_XFER_TIMEOUT 100
@@ -616,11 +600,12 @@ struct em28xx {
enum em28xx_chip_id chip_id;
unsigned int is_em25xx:1; /* em25xx/em276x/7x/8x family bridge */
- unsigned char disconnected:1; /* device has been diconnected */
+ unsigned int disconnected:1;/* device has been diconnected */
unsigned int has_video:1;
unsigned int is_audio_only:1;
unsigned int is_webcam:1;
unsigned int has_msp34xx:1;
+ unsigned int i2c_spee
[PATCH 2/8] media: em28xx: improve the logic with sets Xclk and I2C speed
The logic there should be called on two places. Also,
ideally, it should not be modifying the device struct.
So, change the logic accordingly.
Signed-off-by: Mauro Carvalho Chehab
---
drivers/media/usb/em28xx/em28xx-cards.c | 45 +
1 file changed, 23 insertions(+), 22 deletions(-)
diff --git a/drivers/media/usb/em28xx/em28xx-cards.c
b/drivers/media/usb/em28xx/em28xx-cards.c
index 34e16f6ab4ac..cbd7a43bd559 100644
--- a/drivers/media/usb/em28xx/em28xx-cards.c
+++ b/drivers/media/usb/em28xx/em28xx-cards.c
@@ -2669,20 +2669,35 @@ int em28xx_tuner_callback(void *ptr, int component, int
command, int arg)
}
EXPORT_SYMBOL_GPL(em28xx_tuner_callback);
-static inline void em28xx_set_model(struct em28xx *dev)
+static inline void em28xx_set_xclk_i2c_speed(struct em28xx *dev)
{
- dev->board = em28xx_boards[dev->model];
+ struct em28xx_board *board = &em28xx_boards[dev->model];
+ u8 xclk = board->xclk, i2c_speed = board->i2c_speed;
/* Those are the default values for the majority of boards
Use those values if not specified otherwise at boards entry
*/
- if (!dev->board.xclk)
- dev->board.xclk = EM28XX_XCLK_IR_RC5_MODE |
+ if (!xclk)
+ xclk = EM28XX_XCLK_IR_RC5_MODE |
EM28XX_XCLK_FREQUENCY_12MHZ;
- if (!dev->board.i2c_speed)
- dev->board.i2c_speed = EM28XX_I2C_CLK_WAIT_ENABLE |
- EM28XX_I2C_FREQ_100_KHZ;
+ em28xx_write_reg(dev, EM28XX_R0F_XCLK, xclk);
+
+
+ if (!i2c_speed)
+ i2c_speed = EM28XX_I2C_CLK_WAIT_ENABLE |
+ EM28XX_I2C_FREQ_100_KHZ;
+
+ if (!dev->board.is_em2800)
+ em28xx_write_reg(dev, EM28XX_R06_I2C_CLK, i2c_speed);
+ msleep(50);
+}
+
+static inline void em28xx_set_model(struct em28xx *dev)
+{
+ dev->board = em28xx_boards[dev->model];
+
+ em28xx_set_xclk_i2c_speed(dev);
/* Should be initialized early, for I2C to work */
dev->def_i2c_bus = dev->board.def_i2c_bus;
@@ -2725,10 +2740,7 @@ static void em28xx_pre_card_setup(struct em28xx *dev)
{
/* Set the initial XCLK and I2C clock values based on the board
definition */
- em28xx_write_reg(dev, EM28XX_R0F_XCLK, dev->board.xclk & 0x7f);
- if (!dev->board.is_em2800)
- em28xx_write_reg(dev, EM28XX_R06_I2C_CLK, dev->board.i2c_speed);
- msleep(50);
+ em28xx_set_xclk_i2c_speed(dev);
/* request some modules */
switch (dev->model) {
@@ -3382,17 +3394,6 @@ static int em28xx_init_dev(struct em28xx *dev, struct
usb_device *udev,
em28xx_pre_card_setup(dev);
- if (!dev->board.is_em2800) {
- /* Resets I2C speed */
- retval = em28xx_write_reg(dev, EM28XX_R06_I2C_CLK,
dev->board.i2c_speed);
- if (retval < 0) {
- dev_err(&dev->intf->dev,
- "%s: em28xx_write_reg failed! retval [%d]\n",
- __func__, retval);
- return retval;
- }
- }
-
rt_mutex_init(&dev->i2c_bus_lock);
/* register i2c bus 0 */
--
2.14.3
[PATCH 7/8] media: dvb-core: add helper functions for I2C binding
The dvb_attach()/dvb_detach() methods are ugly hacks designed
to keep using the I2C low-level API. The proper way is to
do I2C bus bindings instead.
Several modules were already converted to use it. Yet,
it is painful to use it, as lots of code need to be
duplicated.
Make it easier by providing two new helper functions:
- dvb_module_probe()
- dvb_module_release()
Signed-off-by: Mauro Carvalho Chehab
---
drivers/media/dvb-core/dvbdev.c | 48 ++
include/media/dvbdev.h | 65 +++--
2 files changed, 111 insertions(+), 2 deletions(-)
diff --git a/drivers/media/dvb-core/dvbdev.c b/drivers/media/dvb-core/dvbdev.c
index 60e9c2ba26be..a840133feacb 100644
--- a/drivers/media/dvb-core/dvbdev.c
+++ b/drivers/media/dvb-core/dvbdev.c
@@ -24,6 +24,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -941,6 +942,53 @@ int dvb_usercopy(struct file *file,
return err;
}
+#ifdef CONFIG_I2C
+struct i2c_client *dvb_module_probe(const char *module_name,
+ const char *name,
+ struct i2c_adapter *adap,
+ unsigned char addr,
+ void *platform_data)
+{
+ struct i2c_client *client;
+ struct i2c_board_info *board_info;
+
+ board_info = kzalloc(sizeof(*board_info), GFP_KERNEL);
+
+ if (name)
+ strlcpy(board_info->type, name, I2C_NAME_SIZE);
+ else
+ strlcpy(board_info->type, module_name, I2C_NAME_SIZE);
+
+ board_info->addr = addr;
+ board_info->platform_data = platform_data;
+ request_module(module_name);
+ client = i2c_new_device(adap, board_info);
+ if (client == NULL || client->dev.driver == NULL) {
+ kfree(board_info);
+ return NULL;
+ }
+
+ if (!try_module_get(client->dev.driver->owner)) {
+ i2c_unregister_device(client);
+ client = NULL;
+ }
+
+ kfree(board_info);
+ return client;
+}
+EXPORT_SYMBOL_GPL(dvb_module_probe);
+
+void dvb_module_release(struct i2c_client *client)
+{
+ if (!client)
+ return;
+
+ module_put(client->dev.driver->owner);
+ i2c_unregister_device(client);
+}
+EXPORT_SYMBOL_GPL(dvb_module_release);
+#endif
+
static int dvb_uevent(struct device *dev, struct kobj_uevent_env *env)
{
struct dvb_device *dvbdev = dev_get_drvdata(dev);
diff --git a/include/media/dvbdev.h b/include/media/dvbdev.h
index 554db879527f..2d2897508590 100644
--- a/include/media/dvbdev.h
+++ b/include/media/dvbdev.h
@@ -358,7 +358,61 @@ long dvb_generic_ioctl(struct file *file,
int dvb_usercopy(struct file *file, unsigned int cmd, unsigned long arg,
int (*func)(struct file *file, unsigned int cmd, void *arg));
-/** generic DVB attach function. */
+#ifdef CONFIG_I2C
+
+struct i2c_adapter;
+struct i2c_client;
+/**
+ * dvb_module_probe - helper routine to probe an I2C module
+ *
+ * @module_name:
+ * Name of the I2C module to be probed
+ * @name:
+ * Optional name for the I2C module. Used for debug purposes.
+ * If %NULL, defaults to @module_name.
+ * @adap:
+ * pointer to &struct i2c_adapter that describes the I2C adapter where
+ * the module will be bound.
+ * @addr:
+ * I2C address of the adapter, in 7-bit notation.
+ * @platform_data:
+ * Platform data to be passed to the I2C module probed.
+ *
+ * This function binds an I2C device into the DVB core. Should be used by
+ * all drivers that use I2C bus to control the hardware. A module bound
+ * with dvb_module_probe() should use dvb_module_release() to unbind.
+ *
+ * Return:
+ * On success, return an &struct i2c_client, pointing the the bound
+ * I2C device. %NULL otherwise.
+ *
+ * .. note::
+ *
+ *In the past, DVB modules (mainly, frontends) were bound via dvb_attach()
+ *macro, with does an ugly hack, using I2C low level functions. Such
+ *usage is deprecated and will be removed soon. Instead, use this routine.
+ */
+struct i2c_client *dvb_module_probe(const char *module_name,
+ const char *name,
+ struct i2c_adapter *adap,
+ unsigned char addr,
+ void *platform_data);
+
+/**
+ * dvb_module_release - releases an I2C device allocated with
+ * dvb_module_probe().
+ *
+ * @client: pointer to &struct i2c_client with the I2C client to be released.
+ * can be %NULL.
+ *
+ * This function should be used to free all resources reserved by
+ * dvb_module_probe() and unbinding the I2C hardware.
+ */
+void dvb_module_release(struct i2c_client *client);
+
+#endif /* CONFIG_I2C */
+
+/* Legacy generic DVB attach function. */
#ifdef CONFIG_MEDIA_ATTACH
/**
@@ -371,6 +425,13 @@ int dvb_usercopy(struct file
[PATCH 0/8] em28xx: some improvements
The first patch in this series change the em28xx to don't require coherent memory for DMA transfers. This should bring some performance gains on non-x86 archs. The other patches in this series are results of my tests with the first patch :-) patch 2 does some cleanups at i2c and Xclock speed settings. Patches 3 and 4 do something that I'm intending to do for a long time: it constifies most static structs inside the driver. Before that, the cards list were modified in runtime, with is a bad idea when multiple (but different) boards use the same entry (for example, a webcam and a em28xx capture "generic" board). Patch 5 solves a FIXME. Patch 6 was written by Arnd, as a way to solve an issue with KASAN: without it, more than 4Kbytes were spent at the stack, with can cause very bad things. He sent it sometime ago. As I answered when the patchset was sent, I didn't like very much that approach, as a way to solve KASAN, as the real solution is to improve the I2C binding logic inside this (and other) drivers. Yet, it brings a nice cleanup at em28xx. So, patch 7 does that! It is something that I'm wanting to do for a long time, but only today I found the needed time: it adds helper functions at the DVB core in order to bind/unbind I2C modules to a driver. It is aligned with the new I2C binding work made by Antti. Patch 8 converts em28xx-dvb to use the new dvb_module_probe() and dvb_module_release() fuctions, with caused a reduction of almost 10% on the size of this module! A side effect of patches 7 and 8 is that we don't to use noinline_for_stack "black" magic there anymore, with is a good thing. Arnd Bergmann (1): media: em28xx: split up em28xx_dvb_init to reduce stack size Mauro Carvalho Chehab (7): media: em28xx: don't use coherent buffer for DMA transfers media: em28xx: improve the logic with sets Xclk and I2C speed media: em28xx: stop rewriting device's struct media: em28xx: constify most static structs media: em28xx: adjust I2C timeout according with I2C speed media: dvb-core: add helper functions for I2C binding media: em28xx-dvb: simplify DVB module probing logic drivers/media/dvb-core/dvbdev.c | 48 ++ drivers/media/usb/em28xx/em28xx-cards.c | 163 +++ drivers/media/usb/em28xx/em28xx-core.c | 57 ++- drivers/media/usb/em28xx/em28xx-dvb.c | 775 drivers/media/usb/em28xx/em28xx-i2c.c | 36 +- drivers/media/usb/em28xx/em28xx-input.c | 6 +- drivers/media/usb/em28xx/em28xx-video.c | 24 +- drivers/media/usb/em28xx/em28xx.h | 39 +- include/media/dvbdev.h | 65 ++- 9 files changed, 579 insertions(+), 634 deletions(-) -- 2.14.3
[PATCH 4/8] media: em28xx: constify most static structs
There are several em28xx static structs that can now be constified.
That caused a significant reduction at data segment:
Before:
textdata bss dec hex filename
85017 59588 576 145181 2371d drivers/media/usb/em28xx/em28xx.o
After:
textdata bss dec hex filename
112345 32292 576 145213 2373d drivers/media/usb/em28xx/em28xx.o
Signed-off-by: Mauro Carvalho Chehab
---
drivers/media/usb/em28xx/em28xx-cards.c | 106
drivers/media/usb/em28xx/em28xx-core.c | 2 +-
drivers/media/usb/em28xx/em28xx-input.c | 6 +-
drivers/media/usb/em28xx/em28xx.h | 16 ++---
4 files changed, 66 insertions(+), 64 deletions(-)
diff --git a/drivers/media/usb/em28xx/em28xx-cards.c
b/drivers/media/usb/em28xx/em28xx-cards.c
index 91c2198c8a2c..02af067d7451 100644
--- a/drivers/media/usb/em28xx/em28xx-cards.c
+++ b/drivers/media/usb/em28xx/em28xx-cards.c
@@ -81,26 +81,26 @@ static void em28xx_pre_card_setup(struct em28xx *dev);
*/
/* Reset for the most [analog] boards */
-static struct em28xx_reg_seq default_analog[] = {
+const static struct em28xx_reg_seq default_analog[] = {
{EM2820_R08_GPIO_CTRL, 0x6d, ~EM_GPIO_4, 10},
{ -1, -1, -1, -1},
};
/* Reset for the most [digital] boards */
-static struct em28xx_reg_seq default_digital[] = {
+const static struct em28xx_reg_seq default_digital[] = {
{EM2820_R08_GPIO_CTRL, 0x6e, ~EM_GPIO_4, 10},
{ -1, -1, -1, -1},
};
/* Board Hauppauge WinTV HVR 900 analog */
-static struct em28xx_reg_seq hauppauge_wintv_hvr_900_analog[] = {
+const static struct em28xx_reg_seq hauppauge_wintv_hvr_900_analog[] = {
{EM2820_R08_GPIO_CTRL, 0x2d, ~EM_GPIO_4, 10},
{ 0x05, 0xff, 0x10, 10},
{ -1, -1, -1, -1},
};
/* Board Hauppauge WinTV HVR 900 digital */
-static struct em28xx_reg_seq hauppauge_wintv_hvr_900_digital[] = {
+const static struct em28xx_reg_seq hauppauge_wintv_hvr_900_digital[] = {
{EM2820_R08_GPIO_CTRL, 0x2e, ~EM_GPIO_4, 10},
{EM2880_R04_GPO,0x04, 0x0f, 10},
{EM2880_R04_GPO,0x0c, 0x0f, 10},
@@ -108,14 +108,14 @@ static struct em28xx_reg_seq
hauppauge_wintv_hvr_900_digital[] = {
};
/* Board Hauppauge WinTV HVR 900 (R2) digital */
-static struct em28xx_reg_seq hauppauge_wintv_hvr_900R2_digital[] = {
+const static struct em28xx_reg_seq hauppauge_wintv_hvr_900R2_digital[] = {
{EM2820_R08_GPIO_CTRL, 0x2e, ~EM_GPIO_4, 10},
{EM2880_R04_GPO,0x0c, 0x0f, 10},
{ -1, -1, -1, -1},
};
/* Boards - EM2880 MSI DIGIVOX AD and EM2880_BOARD_MSI_DIGIVOX_AD_II */
-static struct em28xx_reg_seq em2880_msi_digivox_ad_analog[] = {
+const static struct em28xx_reg_seq em2880_msi_digivox_ad_analog[] = {
{EM2820_R08_GPIO_CTRL, 0x69, ~EM_GPIO_4, 10},
{ -1, -1, -1, -1},
};
@@ -126,7 +126,7 @@ static struct em28xx_reg_seq em2880_msi_digivox_ad_analog[]
= {
Analog - No input analog */
/* Board - EM2882 Kworld 315U digital */
-static struct em28xx_reg_seq em2882_kworld_315u_digital[] = {
+const static struct em28xx_reg_seq em2882_kworld_315u_digital[] = {
{EM2820_R08_GPIO_CTRL, 0xff, 0xff, 10},
{EM2820_R08_GPIO_CTRL, 0xfe, 0xff, 10},
{EM2880_R04_GPO,0x04, 0xff, 10},
@@ -135,7 +135,7 @@ static struct em28xx_reg_seq em2882_kworld_315u_digital[] =
{
{ -1, -1, -1, -1},
};
-static struct em28xx_reg_seq em2882_kworld_315u_tuner_gpio[] = {
+const static struct em28xx_reg_seq em2882_kworld_315u_tuner_gpio[] = {
{EM2880_R04_GPO,0x08, 0xff, 10},
{EM2880_R04_GPO,0x0c, 0xff, 10},
{EM2880_R04_GPO,0x08, 0xff, 10},
@@ -143,13 +143,13 @@ static struct em28xx_reg_seq
em2882_kworld_315u_tuner_gpio[] = {
{ -1, -1, -1, -1},
};
-static struct em28xx_reg_seq kworld_330u_analog[] = {
+const static struct em28xx_reg_seq kworld_330u_analog[] = {
{EM2820_R08_GPIO_CTRL, 0x6d, ~EM_GPIO_4, 10},
{EM2880_R04_GPO,0x00, 0xff, 10},
{ -1, -1, -1, -1},
};
-static struct em28xx_reg_seq kworld_330u_digital[] = {
+const static struct em28xx_reg_seq kworld_330u_digital[] = {
{EM2820_R08_GPIO_CTRL, 0x6e, ~EM_GPIO_4, 10},
{EM2880_R04_GPO,0x08, 0xff, 10},
{ -1, -1, -1, -1},
@@ -161,12 +161,12 @@ static struct em28xx_reg_seq kworld_330u_digital[] = {
GPIO4 - xc3028 reset
GOP3 - s5h1409 reset
*/
-static struct e
Re: [v5 2/2] media: dt-bindings: Add bindings for Dongwoon DW9807 voice coil
On Fri, Mar 02, 2018 at 12:59:00PM -0600, Rob Herring wrote: > On Wed, Feb 28, 2018 at 03:31:26PM +0200, Sakari Ailus wrote: > > Hi Rob, > > > > Thanks for the review. > > > > On Tue, Feb 27, 2018 at 04:10:31PM -0600, Rob Herring wrote: > > > On Fri, Feb 23, 2018 at 10:13 AM, Andy Yeh wrote: > > > > From: Alan Chiang > > > > > > > > Dongwoon DW9807 is a voice coil lens driver. > > > > > > > > Also add a vendor prefix for Dongwoon for one did not exist previously. > > > > > > Where's that? > > > > Added by aece98a912d92444ea9da03b04269407d1308f1f . So that line isn't > > relevant indeed and should be removed. > > > > > > > > > > > > > Signed-off-by: Andy Yeh > > > > --- > > > > Documentation/devicetree/bindings/media/i2c/dongwoon,dw9807.txt | 9 > > > > + > > > > > > DACs generally go in bindings/iio/dac/ > > > > We have quite a few lens voice coil drivers under bindings/media/i2c now. I > > don't really object to putting this one to bindings/iio/dac but then the > > rest should be moved as well. > > > > The camera LED flash drivers are under bindings/leds so this would actually > > be analoguous to that. The lens voice coil drivers are perhaps still a bit > > more bound to the domain (camera) than the LED flash drivers. > > The h/w is bound to that function or just the s/w? The hardware. I guess in principle you could use them for other purposes --- most devices seem to be current sinks with configurable current --- but I've never seen that. The datasheet (dw9714) is here: http://www.datasheetspdf.com/datasheet/download.php?id=840322> > > > I can send a patch if you think the existing bindings should be moved; let > > me know. > > I'm okay if they are separate as long as we're not going to see the > same device show up in both places. However, "i2c" is not the best Ack. I wouldn't expect that. The datasheets of such devices clearly label the devices voice coil module drivers. > directory choice. It should be by function, so we can find common > properties. I2c devices in the media subsystem tend to be peripherals that are always used with another device with access to some system bus. Camera sensors, lens devices and tuners can be found there currently. I don't know the original reasoning but it most likely is related to that. In terms of different kinds of devices we have currently at least the following: Camera ISPs and CSI-2 receivers Video muxes Video codecs Camera sensors Camera lens drivers (right now only voice coil modules?) Tuners (DVB, radio, analogue TV, whatever) Radio transmitters HDMI CEC Remote controllers JPEG codecs Cc Hans, too. -- Sakari Ailus sakari.ai...@linux.intel.com
Re: [PATCH 2/2] media: imx-media-csi: Do not propagate the error when pinctrl is not found
On 03/02/2018 11:14 AM, Fabio Estevam wrote:
From: Fabio Estevam
Since commit 52e17089d185 ("media: imx: Don't initialize vars that
won't be used") imx_csi_probe() fails to probe after propagating the
devm_pinctrl_get_select_default() error.
devm_pinctrl_get_select_default() may return -ENODEV when the CSI pinctrl
entry is not found, so better not to propagate the error in the -ENODEV
case to avoid a regression.
Suggested-by: Philipp Zabel
Signed-off-by: Fabio Estevam
Reviewed-by: Steve Longerbeam
---
drivers/staging/media/imx/imx-media-csi.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/media/imx/imx-media-csi.c
b/drivers/staging/media/imx/imx-media-csi.c
index 4f290a0..5af66f6 100644
--- a/drivers/staging/media/imx/imx-media-csi.c
+++ b/drivers/staging/media/imx/imx-media-csi.c
@@ -1799,7 +1799,10 @@ static int imx_csi_probe(struct platform_device *pdev)
pinctrl = devm_pinctrl_get_select_default(priv->dev);
if (IS_ERR(pinctrl)) {
ret = PTR_ERR(pinctrl);
- goto free;
+ dev_dbg(priv->dev,
+ "devm_pinctrl_get_select_default() failed: %d", ret);
+ if (ret != -ENODEV)
+ goto free;
}
ret = v4l2_async_register_subdev(&priv->sd);
cron job: media_tree daily build: ERRORS
This message is generated daily by a cron job that builds media_tree for the kernels and architectures in the list below. Results of the daily build of media_tree: date: Sat Mar 3 05:00:11 CET 2018 media-tree git hash:e3e389f931a14ddf43089c7db92fc5d74edf93a4 media_build git hash: c3a4fa1a633e24b4a607a78ad11a61598ee177b6 v4l-utils git hash: 5090c6cb0265c6847ff4b0e89b37c172eb0c1c62 gcc version:i686-linux-gcc (GCC) 7.3.0 sparse version: v0.5.0-3994-g45eb2282 smatch version: v0.5.0-3994-g45eb2282 host hardware: x86_64 host os:4.14.0-3-amd64 linux-git-arm-at91: OK linux-git-arm-davinci: OK linux-git-arm-multi: OK linux-git-arm-pxa: OK linux-git-arm-stm32: OK linux-git-arm64: OK linux-git-blackfin-bf561: OK linux-git-i686: OK linux-git-m32r: OK linux-git-mips: OK linux-git-powerpc64: OK linux-git-sh: OK linux-git-x86_64: OK linux-2.6.36.4-i686: ERRORS linux-2.6.36.4-x86_64: ERRORS linux-2.6.37.6-i686: ERRORS linux-2.6.37.6-x86_64: ERRORS linux-2.6.38.8-i686: ERRORS linux-2.6.38.8-x86_64: ERRORS linux-2.6.39.4-i686: ERRORS linux-2.6.39.4-x86_64: ERRORS linux-3.0.60-i686: ERRORS linux-3.0.60-x86_64: ERRORS linux-3.1.10-i686: ERRORS linux-3.1.10-x86_64: ERRORS linux-3.2.98-i686: ERRORS linux-3.2.98-x86_64: ERRORS linux-3.3.8-i686: ERRORS linux-3.3.8-x86_64: ERRORS linux-3.4.27-i686: ERRORS linux-3.4.27-x86_64: ERRORS linux-3.5.7-i686: ERRORS linux-3.5.7-x86_64: ERRORS linux-3.6.11-i686: ERRORS linux-3.6.11-x86_64: ERRORS linux-3.7.4-i686: ERRORS linux-3.7.4-x86_64: ERRORS linux-3.8-i686: ERRORS linux-3.8-x86_64: ERRORS linux-3.9.2-i686: ERRORS linux-3.9.2-x86_64: ERRORS linux-3.10.1-i686: ERRORS linux-3.10.1-x86_64: ERRORS linux-3.11.1-i686: ERRORS linux-3.11.1-x86_64: ERRORS linux-3.12.67-i686: ERRORS linux-3.12.67-x86_64: ERRORS linux-3.13.11-i686: ERRORS linux-3.13.11-x86_64: ERRORS linux-3.14.9-i686: ERRORS linux-3.14.9-x86_64: ERRORS linux-3.15.2-i686: ERRORS linux-3.15.2-x86_64: ERRORS linux-3.16.53-i686: ERRORS linux-3.16.53-x86_64: ERRORS linux-3.17.8-i686: ERRORS linux-3.17.8-x86_64: ERRORS linux-3.18.93-i686: ERRORS linux-3.18.93-x86_64: ERRORS linux-3.19-i686: ERRORS linux-3.19-x86_64: ERRORS linux-4.0.9-i686: ERRORS linux-4.0.9-x86_64: ERRORS linux-4.1.49-i686: ERRORS linux-4.1.49-x86_64: ERRORS linux-4.2.8-i686: ERRORS linux-4.2.8-x86_64: ERRORS linux-4.3.6-i686: WARNINGS linux-4.3.6-x86_64: WARNINGS linux-4.4.115-i686: OK linux-4.4.115-x86_64: OK linux-4.5.7-i686: WARNINGS linux-4.5.7-x86_64: WARNINGS linux-4.6.7-i686: OK linux-4.6.7-x86_64: WARNINGS linux-4.7.5-i686: OK linux-4.7.5-x86_64: WARNINGS linux-4.8-i686: OK linux-4.8-x86_64: WARNINGS linux-4.9.80-i686: OK linux-4.9.80-x86_64: OK linux-4.10.14-i686: OK linux-4.10.14-x86_64: WARNINGS linux-4.11-i686: OK linux-4.11-x86_64: WARNINGS linux-4.12.1-i686: OK linux-4.12.1-x86_64: WARNINGS linux-4.13-i686: OK linux-4.13-x86_64: OK linux-4.14.17-i686: OK linux-4.14.17-x86_64: OK linux-4.15.2-i686: OK linux-4.15.2-x86_64: OK linux-4.16-rc1-i686: OK linux-4.16-rc1-x86_64: OK apps: WARNINGS spec-git: OK sparse: WARNINGS smatch: OK Detailed results are available here: http://www.xs4all.nl/~hverkuil/logs/Saturday.log Full logs are available here: http://www.xs4all.nl/~hverkuil/logs/Saturday.tar.bz2 The Media Infrastructure API from this daily build is here: http://www.xs4all.nl/~hverkuil/spec/index.html
