[PATCH] cec-pin: fix irq handling
The free_irq() function could be called from interrupt context,
which is invalid. Move this to the thread.
In the interrupt handler we just request that the thread disables
the irq. This is done through an atomic so we don't need to add
any spinlocks.
Signed-off-by: Hans Verkuil
---
drivers/media/cec/cec-pin.c | 34 +-
include/media/cec-pin.h | 6 +-
2 files changed, 26 insertions(+), 14 deletions(-)
diff --git a/drivers/media/cec/cec-pin.c b/drivers/media/cec/cec-pin.c
index 03f800e5ec1f..c2454495d562 100644
--- a/drivers/media/cec/cec-pin.c
+++ b/drivers/media/cec/cec-pin.c
@@ -557,7 +557,7 @@ static enum hrtimer_restart cec_pin_timer(struct hrtimer
*timer)
adap->is_configured || adap->monitor_all_cnt)
break;
/* Switch to interrupt mode */
- pin->work_enable_irq = true;
+ atomic_set(&pin->work_irq_change, CEC_PIN_IRQ_ENABLE);
pin->state = CEC_ST_RX_IRQ;
wake_up_interruptible(&pin->kthread_waitq);
return HRTIMER_NORESTART;
@@ -591,7 +591,7 @@ static int cec_pin_thread_func(void *_adap)
kthread_should_stop() ||
pin->work_rx_msg.len ||
pin->work_tx_status ||
- pin->work_enable_irq ||
+ atomic_read(&pin->work_irq_change) ||
atomic_read(&pin->work_pin_events));
if (pin->work_rx_msg.len) {
@@ -606,6 +606,7 @@ static int cec_pin_thread_func(void *_adap)
cec_transmit_attempt_done_ts(adap, tx_status,
pin->work_tx_ts);
}
+
while (atomic_read(&pin->work_pin_events)) {
unsigned int idx = pin->work_pin_events_rd;
@@ -614,14 +615,26 @@ static int cec_pin_thread_func(void *_adap)
pin->work_pin_events_rd = (idx + 1) %
CEC_NUM_PIN_EVENTS;
atomic_dec(&pin->work_pin_events);
}
- if (pin->work_enable_irq) {
- pin->work_enable_irq = false;
+
+ switch (atomic_xchg(&pin->work_irq_change,
+ CEC_PIN_IRQ_UNCHANGED)) {
+ case CEC_PIN_IRQ_DISABLE:
+ pin->ops->disable_irq(adap);
+ cec_pin_high(pin);
+ cec_pin_to_idle(pin);
+ hrtimer_start(&pin->timer, 0, HRTIMER_MODE_REL);
+ break;
+ case CEC_PIN_IRQ_ENABLE:
pin->enable_irq_failed = !pin->ops->enable_irq(adap);
if (pin->enable_irq_failed) {
cec_pin_to_idle(pin);
hrtimer_start(&pin->timer, 0, HRTIMER_MODE_REL);
}
+ break;
+ default:
+ break;
}
+
if (kthread_should_stop())
break;
}
@@ -640,7 +653,7 @@ static int cec_pin_adap_enable(struct cec_adapter *adap,
bool enable)
cec_pin_to_idle(pin);
pin->tx_msg.len = 0;
pin->timer_ts = 0;
- pin->work_enable_irq = false;
+ atomic_set(&pin->work_irq_change, CEC_PIN_IRQ_UNCHANGED);
pin->kthread = kthread_run(cec_pin_thread_func, adap,
"cec-pin");
if (IS_ERR(pin->kthread)) {
@@ -681,7 +694,7 @@ static int cec_pin_adap_transmit(struct cec_adapter *adap,
u8 attempts,
pin->work_tx_status = 0;
pin->tx_bit = 0;
if (pin->state == CEC_ST_RX_IRQ) {
- pin->work_enable_irq = false;
+ atomic_set(&pin->work_irq_change, CEC_PIN_IRQ_UNCHANGED);
pin->ops->disable_irq(adap);
cec_pin_high(pin);
cec_pin_to_idle(pin);
@@ -744,13 +757,8 @@ void cec_pin_changed(struct cec_adapter *adap, bool value)
cec_pin_update(pin, value, false);
if (!value && (adap->is_configuring || adap->is_configured ||
- adap->monitor_all_cnt)) {
- pin->work_enable_irq = false;
- pin->ops->disable_irq(adap);
- cec_pin_high(pin);
- cec_pin_to_idle(pin);
- hrtimer_start(&pin->timer, 0, HRTIMER_MODE_REL);
- }
+ adap->monitor_all_cnt))
+ atomic_set(&pin->work_irq_change, CEC_PIN_IRQ_DISABLE);
}
EXPORT_SYMBOL_GPL(cec_pin_changed);
diff --git a/include/media/cec-pin.h b/include/media/cec-pin.h
index 44b82d29d480..d28d07fa312e 100644
--- a/include/media/cec-pin.h
+++ b/include/media/cec-pin.h
@@ -113,6 +113,10 @@ struct cec_pin_ops {
#define CEC_NUM_PIN_EVENTS 128
+#define CEC_PIN_IRQ_UNCHANGED 0
+#
Re: [PATCHv2 1/3] dt-bindings: document the CEC GPIO bindings
On 08/10/2017 10:33 AM, Hans Verkuil wrote:
> From: Hans Verkuil
>
> Document the bindings for the cec-gpio module for hardware where the
> CEC pin is connected to a GPIO pin.
No need to review this, there will be a v3 that will add a second optional
HPD gpio.
Regards,
Hans
>
> Signed-off-by: Hans Verkuil
> ---
> Documentation/devicetree/bindings/media/cec-gpio.txt | 16
> 1 file changed, 16 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/media/cec-gpio.txt
>
> diff --git a/Documentation/devicetree/bindings/media/cec-gpio.txt
> b/Documentation/devicetree/bindings/media/cec-gpio.txt
> new file mode 100644
> index ..e34a175468e2
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/cec-gpio.txt
> @@ -0,0 +1,16 @@
> +* HDMI CEC GPIO-based hardware
> +
> +Use these bindings for HDMI CEC hardware where the CEC pin is hooked up
> +to a pull-up GPIO pin.
> +
> +Required properties:
> + - compatible: value must be "cec-gpio"
> + - gpio: gpio that the CEC line is connected to
> +
> +Example for the Raspberry Pi 3 where the CEC line is connected to
> +pin 7 aka BCM4 aka GPCLK0 on the GPIO pin header:
> +
> +cec-gpio {
> + compatible = "cec-gpio";
> + gpio = <&gpio 4 GPIO_ACTIVE_HIGH>;
> +};
>
[PATCH 1/1] et8ek8: Decrease stack usage
The et8ek8 driver combines I²C register writes to a single array that it
passes to i2c_transfer(). The maximum number of writes is 48 at once,
decrease it to 8 and make more transfers if needed, thus avoiding a
warning on stack usage.
Signed-off-by: Sakari Ailus
---
Pavel: this is just compile tested. Could you test it on N900, please?
drivers/media/i2c/et8ek8/et8ek8_driver.c | 26 +-
1 file changed, 17 insertions(+), 9 deletions(-)
diff --git a/drivers/media/i2c/et8ek8/et8ek8_driver.c
b/drivers/media/i2c/et8ek8/et8ek8_driver.c
index f39f517..c14f0fd 100644
--- a/drivers/media/i2c/et8ek8/et8ek8_driver.c
+++ b/drivers/media/i2c/et8ek8/et8ek8_driver.c
@@ -43,7 +43,7 @@
#define ET8EK8_NAME"et8ek8"
#define ET8EK8_PRIV_MEM_SIZE 128
-#define ET8EK8_MAX_MSG 48
+#define ET8EK8_MAX_MSG 8
struct et8ek8_sensor {
struct v4l2_subdev subdev;
@@ -220,7 +220,8 @@ static void et8ek8_i2c_create_msg(struct i2c_client
*client, u16 len, u16 reg,
/*
* A buffered write method that puts the wanted register write
- * commands in a message list and passes the list to the i2c framework
+ * commands in smaller number of message lists and passes the lists to
+ * the i2c framework
*/
static int et8ek8_i2c_buffered_write_regs(struct i2c_client *client,
const struct et8ek8_reg *wnext,
@@ -231,11 +232,7 @@ static int et8ek8_i2c_buffered_write_regs(struct
i2c_client *client,
int wcnt = 0;
u16 reg, data_length;
u32 val;
-
- if (WARN_ONCE(cnt > ET8EK8_MAX_MSG,
- ET8EK8_NAME ": %s: too many messages.\n", __func__)) {
- return -EINVAL;
- }
+ int rval;
/* Create new write messages for all writes */
while (wcnt < cnt) {
@@ -249,10 +246,21 @@ static int et8ek8_i2c_buffered_write_regs(struct
i2c_client *client,
/* Update write count */
wcnt++;
+
+ if (wcnt < ET8EK8_MAX_MSG)
+ continue;
+
+ rval = i2c_transfer(client->adapter, msg, wcnt);
+ if (rval < 0)
+ return rval;
+
+ cnt -= wcnt;
+ wcnt = 0;
}
- /* Now we send everything ... */
- return i2c_transfer(client->adapter, msg, wcnt);
+ rval = i2c_transfer(client->adapter, msg, wcnt);
+
+ return rval < 0 ? rval : 0;
}
/*
--
2.7.4
Re: [PATCH v3 14/14] [media] cxd2880 : Update MAINTAINERS file for CXD2880 driver
From: Yasunari Takiguchi I add an e-mail address and re-send this mail again. This is MAINTAINERS file update about the driver for the Sony CXD2880 DVB-T2/T tuner + demodulator. [Change list] Changes in V3 MAINTAINERS -no change Signed-off-by: Yasunari Takiguchi Signed-off-by: Masayuki Yamamoto Signed-off-by: Hideki Nozawa Signed-off-by: Kota Yonezawa Signed-off-by: Toshihiko Matsumoto Signed-off-by: Satoshi Watanabe --- MAINTAINERS | 9 + 1 file changed, 9 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 6f7721d1634c..12a80c33c194 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -8302,6 +8302,15 @@ T: git git://linuxtv.org/media_tree.git S: Supported F: drivers/media/dvb-frontends/cxd2841er* +MEDIA DRIVERS FOR CXD2880 +M: Yasunari Takiguchi +L: linux-media@vger.kernel.org +W: http://linuxtv.org/ +T: git git://linuxtv.org/media_tree.git +S: Supported +F: drivers/media/dvb-frontends/cxd2880/* +F: drivers/media/spi/cxd2880* + MEDIA DRIVERS FOR FREESCALE IMX M: Steve Longerbeam M: Philipp Zabel -- 2.13.0
Re: [PATCH 1/1] et8ek8: Decrease stack usage
On Wed 2017-08-16 10:33:45, Sakari Ailus wrote:
> The et8ek8 driver combines I²C register writes to a single array that it
> passes to i2c_transfer(). The maximum number of writes is 48 at once,
> decrease it to 8 and make more transfers if needed, thus avoiding a
> warning on stack usage.
Dunno. Slowing down code to save stack does not sound attractive.
What about this one? Way simpler, too... (Unless there's some rule
about i2c, DMA and static buffers. Is it?)
Signed-off-by: Pavel Machek
(untested)
Pavel
diff --git a/drivers/media/i2c/et8ek8/et8ek8_driver.c
b/drivers/media/i2c/et8ek8/et8ek8_driver.c
index f39f517..64da731 100644
--- a/drivers/media/i2c/et8ek8/et8ek8_driver.c
+++ b/drivers/media/i2c/et8ek8/et8ek8_driver.c
@@ -227,7 +227,7 @@ static int et8ek8_i2c_buffered_write_regs(struct i2c_client
*client,
int cnt)
{
struct i2c_msg msg[ET8EK8_MAX_MSG];
- unsigned char data[ET8EK8_MAX_MSG][6];
+ static unsigned char data[ET8EK8_MAX_MSG][6];
int wcnt = 0;
u16 reg, data_length;
u32 val;
> ---
> Pavel: this is just compile tested. Could you test it on N900, please?
>
> drivers/media/i2c/et8ek8/et8ek8_driver.c | 26 +-
> 1 file changed, 17 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/media/i2c/et8ek8/et8ek8_driver.c
> b/drivers/media/i2c/et8ek8/et8ek8_driver.c
> index f39f517..c14f0fd 100644
> --- a/drivers/media/i2c/et8ek8/et8ek8_driver.c
> +++ b/drivers/media/i2c/et8ek8/et8ek8_driver.c
> @@ -43,7 +43,7 @@
>
> #define ET8EK8_NAME "et8ek8"
> #define ET8EK8_PRIV_MEM_SIZE 128
> -#define ET8EK8_MAX_MSG 48
> +#define ET8EK8_MAX_MSG 8
>
> struct et8ek8_sensor {
> struct v4l2_subdev subdev;
> @@ -220,7 +220,8 @@ static void et8ek8_i2c_create_msg(struct i2c_client
> *client, u16 len, u16 reg,
>
> /*
> * A buffered write method that puts the wanted register write
> - * commands in a message list and passes the list to the i2c framework
> + * commands in smaller number of message lists and passes the lists to
> + * the i2c framework
> */
> static int et8ek8_i2c_buffered_write_regs(struct i2c_client *client,
> const struct et8ek8_reg *wnext,
> @@ -231,11 +232,7 @@ static int et8ek8_i2c_buffered_write_regs(struct
> i2c_client *client,
> int wcnt = 0;
> u16 reg, data_length;
> u32 val;
> -
> - if (WARN_ONCE(cnt > ET8EK8_MAX_MSG,
> - ET8EK8_NAME ": %s: too many messages.\n", __func__)) {
> - return -EINVAL;
> - }
> + int rval;
>
> /* Create new write messages for all writes */
> while (wcnt < cnt) {
> @@ -249,10 +246,21 @@ static int et8ek8_i2c_buffered_write_regs(struct
> i2c_client *client,
>
> /* Update write count */
> wcnt++;
> +
> + if (wcnt < ET8EK8_MAX_MSG)
> + continue;
> +
> + rval = i2c_transfer(client->adapter, msg, wcnt);
> + if (rval < 0)
> + return rval;
> +
> + cnt -= wcnt;
> + wcnt = 0;
> }
>
> - /* Now we send everything ... */
> - return i2c_transfer(client->adapter, msg, wcnt);
> + rval = i2c_transfer(client->adapter, msg, wcnt);
> +
> + return rval < 0 ? rval : 0;
> }
>
> /*
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures)
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
signature.asc
Description: Digital signature
Re: [PATCH 1/1] et8ek8: Decrease stack usage
On Wed, Aug 16, 2017 at 10:13:05AM +0200, Pavel Machek wrote:
> On Wed 2017-08-16 10:33:45, Sakari Ailus wrote:
> > The et8ek8 driver combines I²C register writes to a single array that it
> > passes to i2c_transfer(). The maximum number of writes is 48 at once,
> > decrease it to 8 and make more transfers if needed, thus avoiding a
> > warning on stack usage.
>
> Dunno. Slowing down code to save stack does not sound attractive.
>
> What about this one? Way simpler, too... (Unless there's some rule
> about i2c, DMA and static buffers. Is it?)
>
> Signed-off-by: Pavel Machek
>
> (untested)
> Pavel
>
> diff --git a/drivers/media/i2c/et8ek8/et8ek8_driver.c
> b/drivers/media/i2c/et8ek8/et8ek8_driver.c
> index f39f517..64da731 100644
> --- a/drivers/media/i2c/et8ek8/et8ek8_driver.c
> +++ b/drivers/media/i2c/et8ek8/et8ek8_driver.c
> @@ -227,7 +227,7 @@ static int et8ek8_i2c_buffered_write_regs(struct
> i2c_client *client,
> int cnt)
> {
> struct i2c_msg msg[ET8EK8_MAX_MSG];
> - unsigned char data[ET8EK8_MAX_MSG][6];
> + static unsigned char data[ET8EK8_MAX_MSG][6];
Works, but we'll need to serialise calls to the function then.
I'm not really sure if passing multiple messages to i2c_transfer() really
even helps here. I think it could be removed altogether as well.
> int wcnt = 0;
> u16 reg, data_length;
> u32 val;
>
>
>
> > ---
> > Pavel: this is just compile tested. Could you test it on N900, please?
> >
> > drivers/media/i2c/et8ek8/et8ek8_driver.c | 26 +-
> > 1 file changed, 17 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/media/i2c/et8ek8/et8ek8_driver.c
> > b/drivers/media/i2c/et8ek8/et8ek8_driver.c
> > index f39f517..c14f0fd 100644
> > --- a/drivers/media/i2c/et8ek8/et8ek8_driver.c
> > +++ b/drivers/media/i2c/et8ek8/et8ek8_driver.c
> > @@ -43,7 +43,7 @@
> >
> > #define ET8EK8_NAME"et8ek8"
> > #define ET8EK8_PRIV_MEM_SIZE 128
> > -#define ET8EK8_MAX_MSG 48
> > +#define ET8EK8_MAX_MSG 8
> >
> > struct et8ek8_sensor {
> > struct v4l2_subdev subdev;
> > @@ -220,7 +220,8 @@ static void et8ek8_i2c_create_msg(struct i2c_client
> > *client, u16 len, u16 reg,
> >
> > /*
> > * A buffered write method that puts the wanted register write
> > - * commands in a message list and passes the list to the i2c framework
> > + * commands in smaller number of message lists and passes the lists to
> > + * the i2c framework
> > */
> > static int et8ek8_i2c_buffered_write_regs(struct i2c_client *client,
> > const struct et8ek8_reg *wnext,
> > @@ -231,11 +232,7 @@ static int et8ek8_i2c_buffered_write_regs(struct
> > i2c_client *client,
> > int wcnt = 0;
> > u16 reg, data_length;
> > u32 val;
> > -
> > - if (WARN_ONCE(cnt > ET8EK8_MAX_MSG,
> > - ET8EK8_NAME ": %s: too many messages.\n", __func__)) {
> > - return -EINVAL;
> > - }
> > + int rval;
> >
> > /* Create new write messages for all writes */
> > while (wcnt < cnt) {
> > @@ -249,10 +246,21 @@ static int et8ek8_i2c_buffered_write_regs(struct
> > i2c_client *client,
> >
> > /* Update write count */
> > wcnt++;
> > +
> > + if (wcnt < ET8EK8_MAX_MSG)
> > + continue;
> > +
> > + rval = i2c_transfer(client->adapter, msg, wcnt);
> > + if (rval < 0)
> > + return rval;
> > +
> > + cnt -= wcnt;
> > + wcnt = 0;
> > }
> >
> > - /* Now we send everything ... */
> > - return i2c_transfer(client->adapter, msg, wcnt);
> > + rval = i2c_transfer(client->adapter, msg, wcnt);
> > +
> > + return rval < 0 ? rval : 0;
> > }
> >
> > /*
>
> --
> (english) http://www.livejournal.com/~pavelmachek
> (cesky, pictures)
> http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
Sakari Ailus
sakari.ai...@linux.intel.com
Re: [PATCH 3/5] s5p-cec: use CEC_CAP_DEFAULTS
On 08/04/2017 12:41 PM, Hans Verkuil wrote: Use the new CEC_CAP_DEFAULTS define in the s5p-cec driver. Signed-off-by: Hans Verkuil Acked-by: Sylwester Nawrocki
Re: [PATCH 1/5] media/cec.h: add CEC_CAP_DEFAULTS
On 08/04/2017 12:41 PM, Hans Verkuil wrote: The CEC_CAP_LOG_ADDRS, CEC_CAP_TRANSMIT, CEC_CAP_PASSTHROUGH and CEC_CAP_RC capabilities are normally always present. Add a CEC_CAP_DEFAULTS define that ORs these four caps to simplify drivers. Signed-off-by: Hans Verkuil Reviewed-by: Sylwester Nawrocki
[PATCH 0/2] [media]: make snd_kcontrol_new const
Make these const. Done using Coccinelle. Bhumika Goyal (2): [media] cx88: make snd_kcontrol_new const [media] solo6x10: make snd_kcontrol_new const drivers/media/pci/cx88/cx88-alsa.c | 2 +- drivers/media/pci/solo6x10/solo6x10-g723.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) -- 1.9.1
[PATCH 2/2] [media] solo6x10: make snd_kcontrol_new const
Make this const as it is only used during a copy operation.
Done using Coccinelle.
Signed-off-by: Bhumika Goyal
---
drivers/media/pci/solo6x10/solo6x10-g723.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/media/pci/solo6x10/solo6x10-g723.c
b/drivers/media/pci/solo6x10/solo6x10-g723.c
index 3ca9470..81be1b8 100644
--- a/drivers/media/pci/solo6x10/solo6x10-g723.c
+++ b/drivers/media/pci/solo6x10/solo6x10-g723.c
@@ -319,7 +319,7 @@ static int snd_solo_capture_volume_put(struct snd_kcontrol
*kcontrol,
return 1;
}
-static struct snd_kcontrol_new snd_solo_capture_volume = {
+static const struct snd_kcontrol_new snd_solo_capture_volume = {
.iface = SNDRV_CTL_ELEM_IFACE_MIXER,
.name = "Capture Volume",
.info = snd_solo_capture_volume_info,
--
1.9.1
[PATCH 1/2] [media] cx88: make snd_kcontrol_new const
Make this const as it only passed as the 1st argument to the function
snd_ctl_new1, which is of type const.
Done using Coccinelle.
Signed-off-by: Bhumika Goyal
---
drivers/media/pci/cx88/cx88-alsa.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/media/pci/cx88/cx88-alsa.c
b/drivers/media/pci/cx88/cx88-alsa.c
index c81fe46..9740326 100644
--- a/drivers/media/pci/cx88/cx88-alsa.c
+++ b/drivers/media/pci/cx88/cx88-alsa.c
@@ -799,7 +799,7 @@ static int snd_cx88_alc_put(struct snd_kcontrol *kcontrol,
return 0;
}
-static struct snd_kcontrol_new snd_cx88_alc_switch = {
+static const struct snd_kcontrol_new snd_cx88_alc_switch = {
.iface = SNDRV_CTL_ELEM_IFACE_MIXER,
.name = "Line-In ALC Switch",
.info = snd_ctl_boolean_mono_info,
--
1.9.1
Re: [PATCH v2] media: isl6421: add checks for current overflow
Em Tue, 15 Aug 2017 20:51:09 +0100
Jemma Denson escreveu:
> Hi Mauro,
>
> On 13/08/17 20:35, Mauro Carvalho Chehab wrote:
>
> > This Kaffeine's BZ:
> > https://bugs.kde.org/show_bug.cgi?id=374693
> >
> > affects SkyStar S2 PCI DVB-S/S2 rev 3.3 device. It could be due to
> > a Kernel bug.
> >
> > While checking the Isil 6421, comparing with its manual, available at:
> >
> > http://www.intersil.com/content/dam/Intersil/documents/isl6/isl6421a.pdf
> >
> > It was noticed that, if the output load is highly capacitive, a different
> > approach
> > is recomended when energizing the LNBf.
> >
> > Also, it is possible to detect if a current overload is happening, by
> > checking an
> > special flag.
> >
> > Add support for it.
> >
> > Compile-tested only.
> >
> > Signed-off-by: Mauro Carvalho Chehab
> > ---
> > drivers/media/dvb-frontends/isl6421.c | 72
> > +--
> > 1 file changed, 68 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/media/dvb-frontends/isl6421.c
> > b/drivers/media/dvb-frontends/isl6421.c
> > index 838b42771a05..b04d56ad4ce8 100644
> > --- a/drivers/media/dvb-frontends/isl6421.c
> > +++ b/drivers/media/dvb-frontends/isl6421.c
> > @@ -38,25 +38,43 @@ struct isl6421 {
> > u8 override_and;
> > struct i2c_adapter *i2c;
> > u8 i2c_addr;
> > + boolis_off;
> > };
> >
> > static int isl6421_set_voltage(struct dvb_frontend *fe,
> >enum fe_sec_voltage voltage)
> > {
> > + int ret;
> > + u8 buf;
> > + bool is_off;
> > struct isl6421 *isl6421 = (struct isl6421 *) fe->sec_priv;
> > - struct i2c_msg msg = { .addr = isl6421->i2c_addr, .flags = 0,
> > - .buf = &isl6421->config,
> > - .len = sizeof(isl6421->config) };
> > + struct i2c_msg msg[2] = {
> > + {
> > + .addr = isl6421->i2c_addr,
> > + .flags = 0,
> > + .buf = &isl6421->config,
> > + .len = 1,
> > + }, {
> > + .addr = isl6421->i2c_addr,
> > + .flags = I2C_M_RD,
> > + .buf = &buf,
> > + .len = 1,
> > + }
> > +
> > + };
> >
> > isl6421->config &= ~(ISL6421_VSEL1 | ISL6421_EN1);
> >
> > switch(voltage) {
> > case SEC_VOLTAGE_OFF:
> > + is_off = true;
> > break;
> > case SEC_VOLTAGE_13:
> > + is_off = false;
> > isl6421->config |= ISL6421_EN1;
> > break;
> > case SEC_VOLTAGE_18:
> > + is_off = false;
> > isl6421->config |= (ISL6421_EN1 | ISL6421_VSEL1);
> > break;
> > default:
> > @@ -66,7 +84,51 @@ static int isl6421_set_voltage(struct dvb_frontend *fe,
> > isl6421->config |= isl6421->override_or;
> > isl6421->config &= isl6421->override_and;
> >
> > - return (i2c_transfer(isl6421->i2c, &msg, 1) == 1) ? 0 : -EIO;
> > + /*
> > +* If LNBf were not powered on, disable dynamic current limit, as,
> > +* according with datasheet, highly capacitive load on the output may
> > +* cause a difficult start-up.
> > +*/
> > + if (isl6421->is_off && !is_off)
> > + isl6421->config |= ISL6421_EN1;
>
> Checking the datasheet I think we need to be setting DCL high instead. EN1 is
> already set anyway.
Yes, that was the idea. Not sure what happened here :-)
> > +
> > + ret = i2c_transfer(isl6421->i2c, msg, 2);
> > + if (ret < 0)
> > + return ret;
> > + if (ret != 2)
> > + return -EIO;
> > +
> > + isl6421->is_off = is_off;
>
> Is this in the right place?
On my first internal versions, this used to be below, but I guess
the best is to place it here, because, if the next i2c_transfer
fail, this would still reflect the current state.
>
> > +
> > + /* On overflow, the device will try again after 900 ms (typically) */
> > + if (isl6421->is_off && (buf & ISL6421_OLF1))
> > + msleep(1000);
>
> 1000ms does only cover one cycle of OFF then ON - the device will keep cycling
> 900ms off then 20ms on until overflow is cleared so it might take longer but
> adding the code to support longer is probably not worth it. Waiting one cycle
> is better than the current none anyway.
Yes, I know it could wait for more time here, but not sure if it would
be worth doing it. The problem is that 1000ms is already a lot of time,
and if this gets too long, userspace may any giving up of tuning,
anyway.
> > +
> > + if (isl6421->is_off && !is_off) {
> > + isl6421->config &= ~ISL6421_EN1;
Again, this should be DCL.
> > +
> > + ret = i2c_transfer(isl6421->i2c, msg, 2);
> > + if (ret < 0)
> > + return ret;
> > + if (ret != 2)
> > + return -EIO;
> > + }
>
> Does this if statement ever match? isl6421->is_off and is_off are the same
> value
> at
[PATCH v2 1/2] v4l: fwnode: Support generic parsing of graph endpoints in a device
The current practice is that drivers iterate over their endpoints and
parse each endpoint separately. This is very similar in a number of
drivers, implement a generic function for the job. Driver specific matters
can be taken into account in the driver specific callback.
Convert the omap3isp as an example.
Signed-off-by: Sakari Ailus
---
drivers/media/platform/omap3isp/isp.c | 116 ++-
drivers/media/platform/omap3isp/isp.h | 3 -
drivers/media/v4l2-core/v4l2-fwnode.c | 125 ++
include/media/v4l2-async.h| 4 +-
include/media/v4l2-fwnode.h | 9 +++
5 files changed, 173 insertions(+), 84 deletions(-)
diff --git a/drivers/media/platform/omap3isp/isp.c
b/drivers/media/platform/omap3isp/isp.c
index 6cb1f0495804..e2f1da966dc8 100644
--- a/drivers/media/platform/omap3isp/isp.c
+++ b/drivers/media/platform/omap3isp/isp.c
@@ -2011,44 +2011,41 @@ enum isp_of_phy {
ISP_OF_PHY_CSIPHY2,
};
-static int isp_fwnode_parse(struct device *dev, struct fwnode_handle *fwnode,
- struct isp_async_subdev *isd)
+static int isp_fwnode_parse(struct device *dev,
+ struct v4l2_fwnode_endpoint *vep,
+ struct v4l2_async_subdev *asd)
{
+ struct isp_async_subdev *isd =
+ container_of(asd, struct isp_async_subdev, asd);
struct isp_bus_cfg *buscfg = &isd->bus;
- struct v4l2_fwnode_endpoint vep;
- unsigned int i;
- int ret;
bool csi1 = false;
-
- ret = v4l2_fwnode_endpoint_parse(fwnode, &vep);
- if (ret)
- return ret;
+ unsigned int i;
dev_dbg(dev, "parsing endpoint %s, interface %u\n",
- to_of_node(fwnode)->full_name, vep.base.port);
+ to_of_node(vep->base.local_fwnode)->full_name, vep->base.port);
- switch (vep.base.port) {
+ switch (vep->base.port) {
case ISP_OF_PHY_PARALLEL:
buscfg->interface = ISP_INTERFACE_PARALLEL;
buscfg->bus.parallel.data_lane_shift =
- vep.bus.parallel.data_shift;
+ vep->bus.parallel.data_shift;
buscfg->bus.parallel.clk_pol =
- !!(vep.bus.parallel.flags
+ !!(vep->bus.parallel.flags
& V4L2_MBUS_PCLK_SAMPLE_FALLING);
buscfg->bus.parallel.hs_pol =
- !!(vep.bus.parallel.flags & V4L2_MBUS_VSYNC_ACTIVE_LOW);
+ !!(vep->bus.parallel.flags &
V4L2_MBUS_VSYNC_ACTIVE_LOW);
buscfg->bus.parallel.vs_pol =
- !!(vep.bus.parallel.flags & V4L2_MBUS_HSYNC_ACTIVE_LOW);
+ !!(vep->bus.parallel.flags &
V4L2_MBUS_HSYNC_ACTIVE_LOW);
buscfg->bus.parallel.fld_pol =
- !!(vep.bus.parallel.flags & V4L2_MBUS_FIELD_EVEN_LOW);
+ !!(vep->bus.parallel.flags & V4L2_MBUS_FIELD_EVEN_LOW);
buscfg->bus.parallel.data_pol =
- !!(vep.bus.parallel.flags & V4L2_MBUS_DATA_ACTIVE_LOW);
- buscfg->bus.parallel.bt656 = vep.bus_type == V4L2_MBUS_BT656;
+ !!(vep->bus.parallel.flags & V4L2_MBUS_DATA_ACTIVE_LOW);
+ buscfg->bus.parallel.bt656 = vep->bus_type == V4L2_MBUS_BT656;
break;
case ISP_OF_PHY_CSIPHY1:
case ISP_OF_PHY_CSIPHY2:
- switch (vep.bus_type) {
+ switch (vep->bus_type) {
case V4L2_MBUS_CCP2:
case V4L2_MBUS_CSI1:
dev_dbg(dev, "CSI-1/CCP-2 configuration\n");
@@ -2060,11 +2057,11 @@ static int isp_fwnode_parse(struct device *dev, struct
fwnode_handle *fwnode,
break;
default:
dev_err(dev, "unsupported bus type %u\n",
- vep.bus_type);
+ vep->bus_type);
return -EINVAL;
}
- switch (vep.base.port) {
+ switch (vep->base.port) {
case ISP_OF_PHY_CSIPHY1:
if (csi1)
buscfg->interface = ISP_INTERFACE_CCP2B_PHY1;
@@ -2080,47 +2077,47 @@ static int isp_fwnode_parse(struct device *dev, struct
fwnode_handle *fwnode,
}
if (csi1) {
buscfg->bus.ccp2.lanecfg.clk.pos =
- vep.bus.mipi_csi1.clock_lane;
+ vep->bus.mipi_csi1.clock_lane;
buscfg->bus.ccp2.lanecfg.clk.pol =
- vep.bus.mipi_csi1.lane_polarity[0];
+ vep->bus.mipi_csi1.lane_polarity[0];
dev_dbg(dev, "clock lane polarity %u, pos %u\n",
buscfg->bus
[PATCH v2 0/2] Unified fwnode endpoint parser
Hi folks, We have a large influx of new, unmerged, drivers that are now parsing fwnode endpoints and each one of them is doing this a little bit differently. The needs are still exactly the same for the graph data structure is device independent. This is still a non-trivial task and the majority of the driver implementations are buggy, just buggy in different ways. Facilitate parsing endpoints by adding a convenience function for parsing the endpoints, and make the omap3isp driver use it as an example. The parser succeeds an essential bugfix in the set. I plan to include the first patch to a pull request soonish, the second could go in with the first user. Open question: should we designate an error code to silently skip endpoints from driver's parse_endpoint() callback? Would that be useful? An error code not relevant for parsing endpoints in general (such as EISDIR) could be chosen, otherwise we're hampering with error codes that can be returned in general. since v1: - The first patch has been merged (it was a bugfix). - In anticipation that the parsing can take place over several iterations, take the existing number of async sub-devices into account when re-allocating an array of async sub-devices. - Rework the first patch to better anticipate parsing single endpoint at a time by a driver. - Add a second patch that adds a function for parsing endpoints one at a time based on port and endpoint numbers. Sakari Ailus (2): v4l: fwnode: Support generic parsing of graph endpoints in a device v4l: fwnode: Support generic parsing of graph endpoints in a single port drivers/media/platform/omap3isp/isp.c | 116 +++--- drivers/media/platform/omap3isp/isp.h | 3 - drivers/media/v4l2-core/v4l2-fwnode.c | 176 ++ include/media/v4l2-async.h| 4 +- include/media/v4l2-fwnode.h | 16 5 files changed, 231 insertions(+), 84 deletions(-) -- 2.11.0
[PATCH v2 2/2] v4l: fwnode: Support generic parsing of graph endpoints in a single port
This is the preferred way to parse the endpoints.
Signed-off-by: Sakari Ailus
---
drivers/media/v4l2-core/v4l2-fwnode.c | 51 +++
include/media/v4l2-fwnode.h | 7 +
2 files changed, 58 insertions(+)
diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c
b/drivers/media/v4l2-core/v4l2-fwnode.c
index cb0fc4b4e3bf..961bcdf22d9a 100644
--- a/drivers/media/v4l2-core/v4l2-fwnode.c
+++ b/drivers/media/v4l2-core/v4l2-fwnode.c
@@ -508,6 +508,57 @@ int v4l2_fwnode_endpoints_parse(
}
EXPORT_SYMBOL_GPL(v4l2_fwnode_endpoints_parse);
+/**
+ * v4l2_fwnode_endpoint_parse - Parse V4L2 fwnode endpoints in a port node
+ * @dev: local struct device
+ * @notifier: async notifier related to @dev
+ * @port: port number
+ * @endpoint: endpoint number
+ * @asd_struct_size: size of the driver's async sub-device struct, including
+ * sizeof(struct v4l2_async_subdev)
+ * @parse_single: driver's callback function called on each V4L2 fwnode
endpoint
+ *
+ * Parse all V4L2 fwnode endpoints related to a given port. This is
+ * the preferred interface over v4l2_fwnode_endpoints_parse() and
+ * should be used by new drivers.
+ */
+int v4l2_fwnode_endpoint_parse_port(
+ struct device *dev, struct v4l2_async_notifier *notifier,
+ unsigned int port, unsigned int endpoint, size_t asd_struct_size,
+ int (*parse_single)(struct device *dev,
+ struct v4l2_fwnode_endpoint *vep,
+ struct v4l2_async_subdev *asd))
+{
+ struct fwnode_handle *fwnode;
+ struct v4l2_async_subdev *asd;
+ int ret;
+
+ fwnode = fwnode_graph_get_remote_node(dev_fwnode(dev), port, endpoint);
+ if (!fwnode)
+ return -ENOENT;
+
+ asd = devm_kzalloc(dev, asd_struct_size, GFP_KERNEL);
+ if (!asd)
+ return -ENOMEM;
+
+ ret = notifier_realloc(dev, notifier, notifier->num_subdevs + 1);
+ if (ret)
+ goto out_free;
+
+ ret = __v4l2_fwnode_endpoint_parse(dev, notifier, fwnode, asd,
+ parse_single);
+ if (ret)
+ goto out_free;
+
+ return 0;
+
+out_free:
+ devm_kfree(dev, asd);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(v4l2_fwnode_endpoint_parse_port);
+
MODULE_LICENSE("GPL");
MODULE_AUTHOR("Sakari Ailus ");
MODULE_AUTHOR("Sylwester Nawrocki ");
diff --git a/include/media/v4l2-fwnode.h b/include/media/v4l2-fwnode.h
index c75a768d4ef7..5adf28e7b070 100644
--- a/include/media/v4l2-fwnode.h
+++ b/include/media/v4l2-fwnode.h
@@ -131,4 +131,11 @@ int v4l2_fwnode_endpoints_parse(
struct v4l2_fwnode_endpoint *vep,
struct v4l2_async_subdev *asd));
+int v4l2_fwnode_endpoint_parse_port(
+ struct device *dev, struct v4l2_async_notifier *notifier,
+ unsigned int port, unsigned int endpoint, size_t asd_struct_size,
+ int (*parse_single)(struct device *dev,
+ struct v4l2_fwnode_endpoint *vep,
+ struct v4l2_async_subdev *asd));
+
#endif /* _V4L2_FWNODE_H */
--
2.11.0
[RESEND PATCH v2 2/2] v4l: fwnode: Support generic parsing of graph endpoints in a single port
This is the preferred way to parse the endpoints.
Signed-off-by: Sakari Ailus
---
drivers/media/v4l2-core/v4l2-fwnode.c | 51 +++
include/media/v4l2-fwnode.h | 7 +
2 files changed, 58 insertions(+)
diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c
b/drivers/media/v4l2-core/v4l2-fwnode.c
index cb0fc4b4e3bf..961bcdf22d9a 100644
--- a/drivers/media/v4l2-core/v4l2-fwnode.c
+++ b/drivers/media/v4l2-core/v4l2-fwnode.c
@@ -508,6 +508,57 @@ int v4l2_fwnode_endpoints_parse(
}
EXPORT_SYMBOL_GPL(v4l2_fwnode_endpoints_parse);
+/**
+ * v4l2_fwnode_endpoint_parse - Parse V4L2 fwnode endpoints in a port node
+ * @dev: local struct device
+ * @notifier: async notifier related to @dev
+ * @port: port number
+ * @endpoint: endpoint number
+ * @asd_struct_size: size of the driver's async sub-device struct, including
+ * sizeof(struct v4l2_async_subdev)
+ * @parse_single: driver's callback function called on each V4L2 fwnode
endpoint
+ *
+ * Parse all V4L2 fwnode endpoints related to a given port. This is
+ * the preferred interface over v4l2_fwnode_endpoints_parse() and
+ * should be used by new drivers.
+ */
+int v4l2_fwnode_endpoint_parse_port(
+ struct device *dev, struct v4l2_async_notifier *notifier,
+ unsigned int port, unsigned int endpoint, size_t asd_struct_size,
+ int (*parse_single)(struct device *dev,
+ struct v4l2_fwnode_endpoint *vep,
+ struct v4l2_async_subdev *asd))
+{
+ struct fwnode_handle *fwnode;
+ struct v4l2_async_subdev *asd;
+ int ret;
+
+ fwnode = fwnode_graph_get_remote_node(dev_fwnode(dev), port, endpoint);
+ if (!fwnode)
+ return -ENOENT;
+
+ asd = devm_kzalloc(dev, asd_struct_size, GFP_KERNEL);
+ if (!asd)
+ return -ENOMEM;
+
+ ret = notifier_realloc(dev, notifier, notifier->num_subdevs + 1);
+ if (ret)
+ goto out_free;
+
+ ret = __v4l2_fwnode_endpoint_parse(dev, notifier, fwnode, asd,
+ parse_single);
+ if (ret)
+ goto out_free;
+
+ return 0;
+
+out_free:
+ devm_kfree(dev, asd);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(v4l2_fwnode_endpoint_parse_port);
+
MODULE_LICENSE("GPL");
MODULE_AUTHOR("Sakari Ailus ");
MODULE_AUTHOR("Sylwester Nawrocki ");
diff --git a/include/media/v4l2-fwnode.h b/include/media/v4l2-fwnode.h
index c75a768d4ef7..5adf28e7b070 100644
--- a/include/media/v4l2-fwnode.h
+++ b/include/media/v4l2-fwnode.h
@@ -131,4 +131,11 @@ int v4l2_fwnode_endpoints_parse(
struct v4l2_fwnode_endpoint *vep,
struct v4l2_async_subdev *asd));
+int v4l2_fwnode_endpoint_parse_port(
+ struct device *dev, struct v4l2_async_notifier *notifier,
+ unsigned int port, unsigned int endpoint, size_t asd_struct_size,
+ int (*parse_single)(struct device *dev,
+ struct v4l2_fwnode_endpoint *vep,
+ struct v4l2_async_subdev *asd));
+
#endif /* _V4L2_FWNODE_H */
--
2.11.0
[RESEND PATCH v2 0/2] Unified fwnode endpoint parser
Hi folks, (Resending, got Niklas's e-mail wrong.) We have a large influx of new, unmerged, drivers that are now parsing fwnode endpoints and each one of them is doing this a little bit differently. The needs are still exactly the same for the graph data structure is device independent. This is still a non-trivial task and the majority of the driver implementations are buggy, just buggy in different ways. Facilitate parsing endpoints by adding a convenience function for parsing the endpoints, and make the omap3isp driver use it as an example. I plan to include the first patch to a pull request soonish, the second could go in with the first user. Open question: should we designate an error code to silently skip endpoints from driver's parse_endpoint() callback? Would that be useful? An error code not relevant for parsing endpoints in general (such as EISDIR) could be chosen, otherwise we're hampering with error codes that can be returned in general. since v1: - The first patch has been merged (it was a bugfix). - In anticipation that the parsing can take place over several iterations, take the existing number of async sub-devices into account when re-allocating an array of async sub-devices. - Rework the first patch to better anticipate parsing single endpoint at a time by a driver. - Add a second patch that adds a function for parsing endpoints one at a time based on port and endpoint numbers. Sakari Ailus (2): v4l: fwnode: Support generic parsing of graph endpoints in a device v4l: fwnode: Support generic parsing of graph endpoints in a single port drivers/media/platform/omap3isp/isp.c | 116 +++--- drivers/media/platform/omap3isp/isp.h | 3 - drivers/media/v4l2-core/v4l2-fwnode.c | 176 ++ include/media/v4l2-async.h| 4 +- include/media/v4l2-fwnode.h | 16 5 files changed, 231 insertions(+), 84 deletions(-) -- 2.11.0
[RESEND PATCH v2 1/2] v4l: fwnode: Support generic parsing of graph endpoints in a device
The current practice is that drivers iterate over their endpoints and
parse each endpoint separately. This is very similar in a number of
drivers, implement a generic function for the job. Driver specific matters
can be taken into account in the driver specific callback.
Convert the omap3isp as an example.
Signed-off-by: Sakari Ailus
---
drivers/media/platform/omap3isp/isp.c | 116 ++-
drivers/media/platform/omap3isp/isp.h | 3 -
drivers/media/v4l2-core/v4l2-fwnode.c | 125 ++
include/media/v4l2-async.h| 4 +-
include/media/v4l2-fwnode.h | 9 +++
5 files changed, 173 insertions(+), 84 deletions(-)
diff --git a/drivers/media/platform/omap3isp/isp.c
b/drivers/media/platform/omap3isp/isp.c
index 6cb1f0495804..e2f1da966dc8 100644
--- a/drivers/media/platform/omap3isp/isp.c
+++ b/drivers/media/platform/omap3isp/isp.c
@@ -2011,44 +2011,41 @@ enum isp_of_phy {
ISP_OF_PHY_CSIPHY2,
};
-static int isp_fwnode_parse(struct device *dev, struct fwnode_handle *fwnode,
- struct isp_async_subdev *isd)
+static int isp_fwnode_parse(struct device *dev,
+ struct v4l2_fwnode_endpoint *vep,
+ struct v4l2_async_subdev *asd)
{
+ struct isp_async_subdev *isd =
+ container_of(asd, struct isp_async_subdev, asd);
struct isp_bus_cfg *buscfg = &isd->bus;
- struct v4l2_fwnode_endpoint vep;
- unsigned int i;
- int ret;
bool csi1 = false;
-
- ret = v4l2_fwnode_endpoint_parse(fwnode, &vep);
- if (ret)
- return ret;
+ unsigned int i;
dev_dbg(dev, "parsing endpoint %s, interface %u\n",
- to_of_node(fwnode)->full_name, vep.base.port);
+ to_of_node(vep->base.local_fwnode)->full_name, vep->base.port);
- switch (vep.base.port) {
+ switch (vep->base.port) {
case ISP_OF_PHY_PARALLEL:
buscfg->interface = ISP_INTERFACE_PARALLEL;
buscfg->bus.parallel.data_lane_shift =
- vep.bus.parallel.data_shift;
+ vep->bus.parallel.data_shift;
buscfg->bus.parallel.clk_pol =
- !!(vep.bus.parallel.flags
+ !!(vep->bus.parallel.flags
& V4L2_MBUS_PCLK_SAMPLE_FALLING);
buscfg->bus.parallel.hs_pol =
- !!(vep.bus.parallel.flags & V4L2_MBUS_VSYNC_ACTIVE_LOW);
+ !!(vep->bus.parallel.flags &
V4L2_MBUS_VSYNC_ACTIVE_LOW);
buscfg->bus.parallel.vs_pol =
- !!(vep.bus.parallel.flags & V4L2_MBUS_HSYNC_ACTIVE_LOW);
+ !!(vep->bus.parallel.flags &
V4L2_MBUS_HSYNC_ACTIVE_LOW);
buscfg->bus.parallel.fld_pol =
- !!(vep.bus.parallel.flags & V4L2_MBUS_FIELD_EVEN_LOW);
+ !!(vep->bus.parallel.flags & V4L2_MBUS_FIELD_EVEN_LOW);
buscfg->bus.parallel.data_pol =
- !!(vep.bus.parallel.flags & V4L2_MBUS_DATA_ACTIVE_LOW);
- buscfg->bus.parallel.bt656 = vep.bus_type == V4L2_MBUS_BT656;
+ !!(vep->bus.parallel.flags & V4L2_MBUS_DATA_ACTIVE_LOW);
+ buscfg->bus.parallel.bt656 = vep->bus_type == V4L2_MBUS_BT656;
break;
case ISP_OF_PHY_CSIPHY1:
case ISP_OF_PHY_CSIPHY2:
- switch (vep.bus_type) {
+ switch (vep->bus_type) {
case V4L2_MBUS_CCP2:
case V4L2_MBUS_CSI1:
dev_dbg(dev, "CSI-1/CCP-2 configuration\n");
@@ -2060,11 +2057,11 @@ static int isp_fwnode_parse(struct device *dev, struct
fwnode_handle *fwnode,
break;
default:
dev_err(dev, "unsupported bus type %u\n",
- vep.bus_type);
+ vep->bus_type);
return -EINVAL;
}
- switch (vep.base.port) {
+ switch (vep->base.port) {
case ISP_OF_PHY_CSIPHY1:
if (csi1)
buscfg->interface = ISP_INTERFACE_CCP2B_PHY1;
@@ -2080,47 +2077,47 @@ static int isp_fwnode_parse(struct device *dev, struct
fwnode_handle *fwnode,
}
if (csi1) {
buscfg->bus.ccp2.lanecfg.clk.pos =
- vep.bus.mipi_csi1.clock_lane;
+ vep->bus.mipi_csi1.clock_lane;
buscfg->bus.ccp2.lanecfg.clk.pol =
- vep.bus.mipi_csi1.lane_polarity[0];
+ vep->bus.mipi_csi1.lane_polarity[0];
dev_dbg(dev, "clock lane polarity %u, pos %u\n",
buscfg->bus
Re: [RFC PATCH] media: vb2: add bidirectional flag in vb2_queue
Hi Hans,
On 08/15/2017 01:04 PM, Hans Verkuil wrote:
> On 08/14/17 10:41, Stanimir Varbanov wrote:
>> Hi,
>>
>> This RFC patch is intended to give to the drivers a choice to change
>> the default behavior of the v4l2-core DMA mapping direction from
>> DMA_TO/FROM_DEVICE (depending on the buffer type CAPTURE or OUTPUT)
>> to DMA_BIDIRECTIONAL during queue_init time.
>>
>> Initially the issue with DMA mapping direction has been found in
>> Venus encoder driver where the firmware side of the driver adds few
>> lines padding on bottom of the image buffer, and the consequence was
>> triggering of IOMMU protection faults.
>>
>> Probably other drivers could also has a benefit of this feature (hint)
>> in the future.
>>
>> Signed-off-by: Stanimir Varbanov
>> ---
>> drivers/media/v4l2-core/videobuf2-core.c | 3 +++
>> include/media/videobuf2-core.h | 11 +++
>> 2 files changed, 14 insertions(+)
>>
>> diff --git a/drivers/media/v4l2-core/videobuf2-core.c
>> b/drivers/media/v4l2-core/videobuf2-core.c
>> index 14f83cecfa92..17d07fda4cdc 100644
>> --- a/drivers/media/v4l2-core/videobuf2-core.c
>> +++ b/drivers/media/v4l2-core/videobuf2-core.c
>> @@ -200,6 +200,9 @@ static int __vb2_buf_mem_alloc(struct vb2_buffer *vb)
>> int plane;
>> int ret = -ENOMEM;
>>
>> +if (q->bidirectional)
>> +dma_dir = DMA_BIDIRECTIONAL;
>> +
>
> Does this only have to be used in mem_alloc? In the __prepare_*() it is still
> using
> DMA_TO/FROM_DEVICE.
Yes, it looks like the DMA direction should be covered in the
__prepare_* too. Thus the patch should look like below:
diff --git a/drivers/media/v4l2-core/videobuf2-core.c
b/drivers/media/v4l2-core/videobuf2-core.c
index 14f83cecfa92..0089e7dac7dd 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -188,14 +188,21 @@ module_param(debug, int, 0644);
static void __vb2_queue_cancel(struct vb2_queue *q);
static void __enqueue_in_driver(struct vb2_buffer *vb);
+static enum dma_data_direction __get_dma_dir(struct vb2_queue *q)
+{
+ if (q->bidirectional)
+ return DMA_BIDIRECTIONAL;
+
+ return q->is_output ? DMA_TO_DEVICE : DMA_FROM_DEVICE;
+}
+
/**
* __vb2_buf_mem_alloc() - allocate video memory for the given buffer
*/
static int __vb2_buf_mem_alloc(struct vb2_buffer *vb)
{
struct vb2_queue *q = vb->vb2_queue;
- enum dma_data_direction dma_dir =
- q->is_output ? DMA_TO_DEVICE : DMA_FROM_DEVICE;
+ enum dma_data_direction dma_dir = __get_dma_dir(q);
void *mem_priv;
int plane;
int ret = -ENOMEM;
@@ -978,8 +985,7 @@ static int __prepare_userptr(struct vb2_buffer *vb,
const void *pb)
void *mem_priv;
unsigned int plane;
int ret = 0;
- enum dma_data_direction dma_dir =
- q->is_output ? DMA_TO_DEVICE : DMA_FROM_DEVICE;
+ enum dma_data_direction dma_dir = __get_dma_dir(q);
bool reacquired = vb->planes[0].mem_priv == NULL;
memset(planes, 0, sizeof(planes[0]) * vb->num_planes);
@@ -1096,8 +1102,7 @@ static int __prepare_dmabuf(struct vb2_buffer *vb,
const void *pb)
void *mem_priv;
unsigned int plane;
int ret = 0;
- enum dma_data_direction dma_dir =
- q->is_output ? DMA_TO_DEVICE : DMA_FROM_DEVICE;
+ enum dma_data_direction dma_dir = __get_dma_dir(q);
bool reacquired = vb->planes[0].mem_priv == NULL;
memset(planes, 0, sizeof(planes[0]) * vb->num_planes);
--
regards,
Stan
Re: [PATCH] drm/bridge/sii8620: add remote control support
On 03.08.2017 10:28, Hans Verkuil wrote:
> Hi Maciej,
>
> Unfortunately I do not have the MHL spec, but I was wondering what the
> relationship between RCP and CEC is. CEC has remote control support as
> well, so is RCP that subset of the CEC specification or is it completely
> separate?
We also do not have MHL specs. From my research it looks like MHL
consortium was mainly focused on supporting different input devices -
remote control, mice, keyboard, touchscreen, game controller, etc. In
public data sheets of some chips Lattice/Silicon Image (main MHL chip
producer) suggest they do not support CEC pass-through via MHL[1].
On the other side superMHL extends RCP with support for multiple devices
[2], so for me it looks like RCP wants to be an alternative to CEC.
But all this is just my interpretation of info found on the Net.
[1]:
http://www.latticesemi.com/~/media/LatticeSemi/Documents/DataSheets/ASSP/SiI-DS-1128_Public.pdf?document_id=51627
[2]: https://en.wikipedia.org/wiki/Mobile_High-Definition_Link#superMHL
Regards
Andrzej
>
> I'm CC-ing Sean Young and the linux-media mailinglist as well since Sean
> maintains the rc subsystem. Which you probably should use, but I'm not the
> expert on that.
>
> Regards,
>
> Hans
>
> On 08/03/17 09:44, Maciej Purski wrote:
>> MHL specification defines Remote Control Protocol(RCP) to
>> send input events between MHL devices.
>> The driver now recognizes RCP messages and reacts to them
>> by reporting key events to input subsystem, allowing
>> a user to control a device using TV remote control.
>>
>> Signed-off-by: Maciej Purski
>> ---
>> drivers/gpu/drm/bridge/sil-sii8620.c | 188
>> ++-
>> include/drm/bridge/mhl.h | 4 +
>> 2 files changed, 187 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/bridge/sil-sii8620.c
>> b/drivers/gpu/drm/bridge/sil-sii8620.c
>> index 2d51a22..7e75f2f 100644
>> --- a/drivers/gpu/drm/bridge/sil-sii8620.c
>> +++ b/drivers/gpu/drm/bridge/sil-sii8620.c
>> @@ -19,6 +19,7 @@
>> #include
>> #include
>> #include
>> +#include
>> #include
>> #include
>> #include
>> @@ -58,6 +59,7 @@ enum sii8620_mt_state {
>> struct sii8620 {
>> struct drm_bridge bridge;
>> struct device *dev;
>> +struct input_dev *rcp_input_dev;
>> struct clk *clk_xtal;
>> struct gpio_desc *gpio_reset;
>> struct gpio_desc *gpio_int;
>> @@ -106,6 +108,82 @@ struct sii8620_mt_msg {
>> sii8620_cb continuation;
>> };
>>
>> +static struct {
>> +u16 key;
>> +u16 extra_key;
>> +bool autorepeat;
>> +} rcp_keymap[] = {
>> +[0x00] = { KEY_SELECT },
>> +[0x01] = { KEY_UP, 0, true },
>> +[0x02] = { KEY_DOWN, 0, true },
>> +[0x03] = { KEY_LEFT, 0, true },
>> +[0x04] = { KEY_RIGHT, 0, true },
>> +
>> +[0x05] = { KEY_RIGHT, KEY_UP, true },
>> +[0x06] = { KEY_RIGHT, KEY_DOWN, true },
>> +[0x07] = { KEY_LEFT, KEY_UP, true },
>> +[0x08] = { KEY_LEFT, KEY_DOWN, true },
>> +
>> +[0x09] = { KEY_MENU },
>> +[0x0A] = { KEY_UNKNOWN },
>> +[0x0B] = { KEY_UNKNOWN },
>> +[0x0C] = { KEY_BOOKMARKS },
>> +[0x0D] = { KEY_EXIT },
>> +
>> +[0x20] = { KEY_NUMERIC_0 },
>> +[0x21] = { KEY_NUMERIC_1 },
>> +[0x22] = { KEY_NUMERIC_2 },
>> +[0x23] = { KEY_NUMERIC_3 },
>> +[0x24] = { KEY_NUMERIC_4 },
>> +[0x25] = { KEY_NUMERIC_5 },
>> +[0x26] = { KEY_NUMERIC_6 },
>> +[0x27] = { KEY_NUMERIC_7 },
>> +[0x28] = { KEY_NUMERIC_8 },
>> +[0x29] = { KEY_NUMERIC_9 },
>> +
>> +[0x2A] = { KEY_DOT },
>> +[0x2B] = { KEY_ENTER },
>> +[0x2C] = { KEY_CLEAR },
>> +
>> +[0x30] = { KEY_CHANNELUP, 0, true },
>> +[0x31] = { KEY_CHANNELDOWN, 0, true },
>> +
>> +[0x33] = { KEY_SOUND },
>> +[0x35] = { KEY_PROGRAM }, /* Show Information */
>> +
>> +[0x37] = { KEY_PAGEUP, 0, true },
>> +[0x38] = { KEY_PAGEDOWN, 0, true },
>> +
>> +[0x41] = { KEY_VOLUMEUP, 0, true },
>> +[0x42] = { KEY_VOLUMEDOWN, 0, true },
>> +[0x43] = { KEY_MUTE },
>> +[0x44] = { KEY_PLAY },
>> +[0x45] = { KEY_STOP },
>> +[0x46] = { KEY_PLAYPAUSE }, /* Pause */
>> +[0x47] = { KEY_RECORD },
>> +[0x48] = { KEY_REWIND, 0, true },
>> +[0x49] = { KEY_FASTFORWARD, 0, true },
>> +[0x4A] = { KEY_EJECTCD },
>> +[0x4B] = { KEY_NEXTSONG, 0, true }, /* Forward */
>> +[0x4C] = { KEY_PREVIOUSSONG, 0, true }, /* Backward */
>> +
>> +[0x60] = { KEY_PLAYPAUSE }, /* Play */
>> +[0x61] = { KEY_PLAYPAUSE }, /* Pause the Play */
>> +[0x62] = { KEY_RECORD },
>> +[0x63] = { KEY_PAUSE },
>> +[0x64] = { KEY_STOP },
>> +[0x65] = { KEY_MUTE },
>> +[0x66] = { KEY_MUTE }, /* Restore Mute */
>> +
>> +[0x71] = { KEY_F1 },
>> +[0x72] = { KEY_F2 },
>> +[0x73] = { KEY_F3 },
>> +[0x74] = { KEY_F4 },
>> +[0x75] = { KEY_F5 },
>> +
>> +[0x7E] = { KEY_VENDOR },
>> +};
>> +
>> static const u8 sii8620_i2c_page[] = {
>> 0x39, /* Main System */
>>
[PATCH v2 0/2] Document s_stream video op calling (MC only) and CSI-2 stream stopping
Hi folks, I've updated the patch documenting the s_stream() video op calling for MC enabled devices based on the review comments. since v1: - Split "stopping the transmitter" documentation to a separate patch and move it to csi2.rst which is a better place for it. - Precise that the added s_stream() video op documentation only applies to Media controller enabled devices. - Better wording for the note which discourages deep recursion in pipeline start / stop. Sakari Ailus (2): docs-rst: media: Document s_stream() video op usage for MC enabled devices docs-rst: media: Document broken frame handling in stream stop for CSI-2 Documentation/media/kapi/csi2.rst| 10 ++ Documentation/media/kapi/v4l2-subdev.rst | 29 + 2 files changed, 39 insertions(+) -- 2.7.4
[PATCH v2 2/2] docs-rst: media: Document broken frame handling in stream stop for CSI-2
Some CSI-2 transmitters will finish an ongoing frame whereas others will not. Document that receiver drivers should not assume a particular behaviour but to work in both cases. Signed-off-by: Sakari Ailus --- Documentation/media/kapi/csi2.rst | 10 ++ 1 file changed, 10 insertions(+) diff --git a/Documentation/media/kapi/csi2.rst b/Documentation/media/kapi/csi2.rst index e33fcb9..0560100 100644 --- a/Documentation/media/kapi/csi2.rst +++ b/Documentation/media/kapi/csi2.rst @@ -51,6 +51,16 @@ not active. Some transmitters do this automatically but some have to be explicitly programmed to do so, and some are unable to do so altogether due to hardware constraints. +Stopping the transmitter + + +A transmitter stops sending the stream of images as a result of +calling the ``.s_stream()`` callback. Some transmitters may stop the +stream at a frame boundary whereas others stop immediately, +effectively leaving the current frame unfinished. The receiver driver +should not make assumptions either way, but function properly in both +cases. + Receiver drivers -- 2.7.4
[PATCH v2 1/2] docs-rst: media: Document s_stream() video op usage for MC enabled devices
As we begin to add support for systems with Media controller pipelines controlled by more than one device driver, it is essential that we precisely define the responsibilities of each component in the stream control and common practices. Specifically, streaming control is done per sub-device and sub-device drivers themselves are responsible for streaming setup in upstream sub-devices. Signed-off-by: Sakari Ailus Acked-by: Niklas Söderlund --- Documentation/media/kapi/v4l2-subdev.rst | 29 + 1 file changed, 29 insertions(+) diff --git a/Documentation/media/kapi/v4l2-subdev.rst b/Documentation/media/kapi/v4l2-subdev.rst index e1f0b72..45088ad 100644 --- a/Documentation/media/kapi/v4l2-subdev.rst +++ b/Documentation/media/kapi/v4l2-subdev.rst @@ -262,6 +262,35 @@ is called. After all subdevices have been located the .complete() callback is called. When a subdevice is removed from the system the .unbind() method is called. All three callbacks are optional. +Streaming control on Media controller enabled devices +^ + +Starting and stopping the stream are somewhat complex operations that +often require walking the media graph to enable streaming on +sub-devices which the pipeline consists of. This involves interaction +between multiple drivers, sometimes more than two. + +The ``.s_stream()`` op in :c:type:`v4l2_subdev_video_ops` is responsible +for starting and stopping the stream on the sub-device it is called +on. A device driver is only responsible for calling the ``.s_stream()`` ops +of the adjacent sub-devices that are connected to its sink pads +through an enabled link. A driver may not call ``.s_stream()`` op +of any other sub-device further up in the pipeline, for instance. + +This means that a sub-device driver is thus in direct control of +whether the upstream sub-devices start (or stop) streaming before or +after the sub-device itself is set up for streaming. + +.. note:: + + As the ``.s_stream()`` callback is called recursively through the + sub-devices along the pipeline, it is important to keep the + recursion as short as possible. To this end, drivers are encouraged + to avoid recursively calling ``.s_stream()`` internally to reduce + stack usage. Instead, the ``.s_stream()`` op of the directly + connected sub-devices should come from the callback through which + the driver was first called. + V4L2 sub-device userspace API - -- 2.7.4
Re: [RFC PATCH] media: vb2: add bidirectional flag in vb2_queue
Hi Stan,
On Wednesday 16 Aug 2017 14:46:50 Stanimir Varbanov wrote:
> On 08/15/2017 01:04 PM, Hans Verkuil wrote:
> > On 08/14/17 10:41, Stanimir Varbanov wrote:
> >> Hi,
> >>
> >> This RFC patch is intended to give to the drivers a choice to change
> >> the default behavior of the v4l2-core DMA mapping direction from
> >> DMA_TO/FROM_DEVICE (depending on the buffer type CAPTURE or OUTPUT)
> >> to DMA_BIDIRECTIONAL during queue_init time.
> >>
> >> Initially the issue with DMA mapping direction has been found in
> >> Venus encoder driver where the firmware side of the driver adds few
> >> lines padding on bottom of the image buffer, and the consequence was
> >> triggering of IOMMU protection faults.
> >>
> >> Probably other drivers could also has a benefit of this feature (hint)
> >> in the future.
> >>
> >> Signed-off-by: Stanimir Varbanov
> >> ---
> >>
> >> drivers/media/v4l2-core/videobuf2-core.c | 3 +++
> >> include/media/videobuf2-core.h | 11 +++
> >> 2 files changed, 14 insertions(+)
> >>
> >> diff --git a/drivers/media/v4l2-core/videobuf2-core.c
> >> b/drivers/media/v4l2-core/videobuf2-core.c index
> >> 14f83cecfa92..17d07fda4cdc 100644
> >> --- a/drivers/media/v4l2-core/videobuf2-core.c
> >> +++ b/drivers/media/v4l2-core/videobuf2-core.c
> >> @@ -200,6 +200,9 @@ static int __vb2_buf_mem_alloc(struct vb2_buffer *vb)
> >>
> >>int plane;
> >>int ret = -ENOMEM;
> >>
> >> + if (q->bidirectional)
> >> + dma_dir = DMA_BIDIRECTIONAL;
> >> +
> >
> > Does this only have to be used in mem_alloc? In the __prepare_*() it is
> > still using DMA_TO/FROM_DEVICE.
>
> Yes, it looks like the DMA direction should be covered in the
> __prepare_* too. Thus the patch should look like below:
>
> diff --git a/drivers/media/v4l2-core/videobuf2-core.c
> b/drivers/media/v4l2-core/videobuf2-core.c
> index 14f83cecfa92..0089e7dac7dd 100644
> --- a/drivers/media/v4l2-core/videobuf2-core.c
> +++ b/drivers/media/v4l2-core/videobuf2-core.c
> @@ -188,14 +188,21 @@ module_param(debug, int, 0644);
> static void __vb2_queue_cancel(struct vb2_queue *q);
> static void __enqueue_in_driver(struct vb2_buffer *vb);
>
> +static enum dma_data_direction __get_dma_dir(struct vb2_queue *q)
> +{
> + if (q->bidirectional)
> + return DMA_BIDIRECTIONAL;
> +
> + return q->is_output ? DMA_TO_DEVICE : DMA_FROM_DEVICE;
We could also compute the DMA direction once only and store it in the queue. I
have no big preference at the moment.
> +}
> +
> /**
> * __vb2_buf_mem_alloc() - allocate video memory for the given buffer
> */
> static int __vb2_buf_mem_alloc(struct vb2_buffer *vb)
> {
> struct vb2_queue *q = vb->vb2_queue;
> - enum dma_data_direction dma_dir =
> - q->is_output ? DMA_TO_DEVICE : DMA_FROM_DEVICE;
> + enum dma_data_direction dma_dir = __get_dma_dir(q);
> void *mem_priv;
> int plane;
> int ret = -ENOMEM;
> @@ -978,8 +985,7 @@ static int __prepare_userptr(struct vb2_buffer *vb,
> const void *pb)
> void *mem_priv;
> unsigned int plane;
> int ret = 0;
> - enum dma_data_direction dma_dir =
> - q->is_output ? DMA_TO_DEVICE : DMA_FROM_DEVICE;
> + enum dma_data_direction dma_dir = __get_dma_dir(q);
> bool reacquired = vb->planes[0].mem_priv == NULL;
>
> memset(planes, 0, sizeof(planes[0]) * vb->num_planes);
> @@ -1096,8 +1102,7 @@ static int __prepare_dmabuf(struct vb2_buffer *vb,
> const void *pb)
> void *mem_priv;
> unsigned int plane;
> int ret = 0;
> - enum dma_data_direction dma_dir =
> - q->is_output ? DMA_TO_DEVICE : DMA_FROM_DEVICE;
> + enum dma_data_direction dma_dir = __get_dma_dir(q);
> bool reacquired = vb->planes[0].mem_priv == NULL;
>
> memset(planes, 0, sizeof(planes[0]) * vb->num_planes);
--
Regards,
Laurent Pinchart
Re: [PATCH v2 0/2] Unified fwnode endpoint parser
The patches depend on the ccp2 patches here: https://git.linuxtv.org/sailus/media_tree.git/log/?h=ccp2> On Wed, Aug 16, 2017 at 02:20:17PM +0300, Sakari Ailus wrote: > Hi folks, > > We have a large influx of new, unmerged, drivers that are now parsing > fwnode endpoints and each one of them is doing this a little bit > differently. The needs are still exactly the same for the graph data > structure is device independent. This is still a non-trivial task and the > majority of the driver implementations are buggy, just buggy in different > ways. > > Facilitate parsing endpoints by adding a convenience function for parsing > the endpoints, and make the omap3isp driver use it as an example. > > The parser succeeds an essential bugfix in the set. > > I plan to include the first patch to a pull request soonish, the second > could go in with the first user. > > Open question: should we designate an error code to silently skip endpoints > from driver's parse_endpoint() callback? Would that be useful? An error > code not relevant for parsing endpoints in general (such as EISDIR) could > be chosen, otherwise we're hampering with error codes that can be returned > in general. > > since v1: > > - The first patch has been merged (it was a bugfix). > > - In anticipation that the parsing can take place over several iterations, > take the existing number of async sub-devices into account when > re-allocating an array of async sub-devices. > > - Rework the first patch to better anticipate parsing single endpoint at a > time by a driver. > > - Add a second patch that adds a function for parsing endpoints one at a > time based on port and endpoint numbers. > > Sakari Ailus (2): > v4l: fwnode: Support generic parsing of graph endpoints in a device > v4l: fwnode: Support generic parsing of graph endpoints in a single > port > > drivers/media/platform/omap3isp/isp.c | 116 +++--- > drivers/media/platform/omap3isp/isp.h | 3 - > drivers/media/v4l2-core/v4l2-fwnode.c | 176 > ++ > include/media/v4l2-async.h| 4 +- > include/media/v4l2-fwnode.h | 16 > 5 files changed, 231 insertions(+), 84 deletions(-) > > -- > 2.11.0 > > -- Sakari Ailus e-mail: sakari.ai...@iki.fi
[PATCH v2 0/5] Omap3isp CCP2 support
Hi folks,
Here's a respin of the omap3isp ccp2 support patches.
since v1:
- Root out patches already merged or not needed (omapisp: Ignore endpoints
with invalid configuration).
- Rework the patch skipping CSI-2 receiver configuration for CCP2
("omap3isp: Skip CSI-2 receiver initialisation in CCP2 configuration").
Instead of trying to figure out which receiver (CSI-2 or CCP2) is used,
store the information to the isp_csiphy struct instead.
- Added a patch to correctly release a CSI-2 phy --- the PHY configuration
coming from DT was previously ignored.
Pavel Machek (2):
omap3isp: Parse CSI1 configuration from the device tree
omap3isp: Correctly set IO_OUT_SEL and VP_CLK_POL for CCP2 mode
Sakari Ailus (3):
omap3isp: Always initialise isp and mutex for csiphy1
omap3isp: csiphy: Don't assume the CSI receiver is a CSI2 module
omap3isp: Correctly configure routing in PHY release
drivers/media/platform/omap3isp/isp.c | 105 +---
drivers/media/platform/omap3isp/ispccp2.c | 9 ++-
drivers/media/platform/omap3isp/ispcsi2.c | 4 +-
drivers/media/platform/omap3isp/ispcsiphy.c | 59
drivers/media/platform/omap3isp/ispcsiphy.h | 6 +-
drivers/media/platform/omap3isp/ispreg.h| 4 ++
drivers/media/platform/omap3isp/omap3isp.h | 1 +
7 files changed, 126 insertions(+), 62 deletions(-)
--
2.11.0
[PATCH v2 3/5] omap3isp: Always initialise isp and mutex for csiphy1
The PHY is still relevant for CCP2.
Signed-off-by: Sakari Ailus
Tested-by: Laurent Pinchart # on
Beagleboard-xM + MPT9P031
---
drivers/media/platform/omap3isp/ispcsiphy.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/media/platform/omap3isp/ispcsiphy.c
b/drivers/media/platform/omap3isp/ispcsiphy.c
index addc6efbb033..2028bb519108 100644
--- a/drivers/media/platform/omap3isp/ispcsiphy.c
+++ b/drivers/media/platform/omap3isp/ispcsiphy.c
@@ -345,13 +345,14 @@ int omap3isp_csiphy_init(struct isp_device *isp)
phy2->phy_regs = OMAP3_ISP_IOMEM_CSIPHY2;
mutex_init(&phy2->mutex);
+ phy1->isp = isp;
+ mutex_init(&phy1->mutex);
+
if (isp->revision == ISP_REVISION_15_0) {
- phy1->isp = isp;
phy1->csi2 = &isp->isp_csi2c;
phy1->num_data_lanes = ISP_CSIPHY1_NUM_DATA_LANES;
phy1->cfg_regs = OMAP3_ISP_IOMEM_CSI2C_REGS1;
phy1->phy_regs = OMAP3_ISP_IOMEM_CSIPHY1;
- mutex_init(&phy1->mutex);
}
return 0;
--
2.11.0
[PATCH v2 5/5] omap3isp: Correctly configure routing in PHY release
The PHY configuration was obtained from DT when the PHY was acquired but
the same was not done when it was released. Fix this.
Signed-off-by: Sakari Ailus
---
drivers/media/platform/omap3isp/ispcsiphy.c | 19 +--
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/drivers/media/platform/omap3isp/ispcsiphy.c
b/drivers/media/platform/omap3isp/ispcsiphy.c
index ed1eb9907ae0..45ed1adbd9ae 100644
--- a/drivers/media/platform/omap3isp/ispcsiphy.c
+++ b/drivers/media/platform/omap3isp/ispcsiphy.c
@@ -155,6 +155,17 @@ static int csiphy_set_power(struct isp_csiphy *phy, u32
power)
return 0;
}
+static struct isp_bus_cfg *omap3isp_csiphy_get_phy_cfg(
+ struct isp_csiphy *phy)
+{
+ struct isp_pipeline *pipe = to_isp_pipeline(phy->entity);
+ struct isp_async_subdev *isd =
+ container_of(pipe->external->asd, struct isp_async_subdev, asd);
+
+ return pipe->external->host_priv ?
+ pipe->external->host_priv : &isd->bus;
+}
+
/*
* TCLK values are OK at their reset values
*/
@@ -165,10 +176,7 @@ static int csiphy_set_power(struct isp_csiphy *phy, u32
power)
static int omap3isp_csiphy_config(struct isp_csiphy *phy)
{
struct isp_pipeline *pipe = to_isp_pipeline(phy->entity);
- struct isp_async_subdev *isd =
- container_of(pipe->external->asd, struct isp_async_subdev, asd);
- struct isp_bus_cfg *buscfg = pipe->external->host_priv ?
- pipe->external->host_priv : &isd->bus;
+ struct isp_bus_cfg *buscfg = omap3isp_csiphy_get_phy_cfg(phy);
struct isp_csiphy_lanes_cfg *lanes;
int csi2_ddrclk_khz;
unsigned int num_data_lanes, used_lanes = 0;
@@ -310,8 +318,7 @@ void omap3isp_csiphy_release(struct isp_csiphy *phy)
{
mutex_lock(&phy->mutex);
if (phy->entity) {
- struct isp_pipeline *pipe = to_isp_pipeline(phy->entity);
- struct isp_bus_cfg *buscfg = pipe->external->host_priv;
+ struct isp_bus_cfg *buscfg = omap3isp_csiphy_get_phy_cfg(phy);
csiphy_routing_cfg(phy, buscfg->interface, false,
buscfg->bus.ccp2.phy_layer);
--
2.11.0
[PATCH v2 2/5] omap3isp: Correctly set IO_OUT_SEL and VP_CLK_POL for CCP2 mode
From: Pavel Machek ISP CSI1 module needs all the bits correctly set to work. Signed-off-by: Ivaylo Dimitrov Signed-off-by: Pavel Machek Signed-off-by: Sakari Ailus Tested-by: Laurent Pinchart # on Beagleboard-xM + MPT9P031 --- drivers/media/platform/omap3isp/ispccp2.c | 7 +-- drivers/media/platform/omap3isp/ispreg.h | 4 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/drivers/media/platform/omap3isp/ispccp2.c b/drivers/media/platform/omap3isp/ispccp2.c index 8b6f7d2e79a0..47210b102bcb 100644 --- a/drivers/media/platform/omap3isp/ispccp2.c +++ b/drivers/media/platform/omap3isp/ispccp2.c @@ -213,14 +213,17 @@ static int ccp2_phyif_config(struct isp_ccp2_device *ccp2, struct isp_device *isp = to_isp_device(ccp2); u32 val; - /* CCP2B mode */ val = isp_reg_readl(isp, OMAP3_ISP_IOMEM_CCP2, ISPCCP2_CTRL) | - ISPCCP2_CTRL_IO_OUT_SEL | ISPCCP2_CTRL_MODE; + ISPCCP2_CTRL_MODE; /* Data/strobe physical layer */ BIT_SET(val, ISPCCP2_CTRL_PHY_SEL_SHIFT, ISPCCP2_CTRL_PHY_SEL_MASK, buscfg->phy_layer); + BIT_SET(val, ISPCCP2_CTRL_IO_OUT_SEL_SHIFT, + ISPCCP2_CTRL_IO_OUT_SEL_MASK, buscfg->ccp2_mode); BIT_SET(val, ISPCCP2_CTRL_INV_SHIFT, ISPCCP2_CTRL_INV_MASK, buscfg->strobe_clk_pol); + BIT_SET(val, ISPCCP2_CTRL_VP_CLK_POL_SHIFT, + ISPCCP2_CTRL_VP_CLK_POL_MASK, buscfg->vp_clk_pol); isp_reg_writel(isp, val, OMAP3_ISP_IOMEM_CCP2, ISPCCP2_CTRL); val = isp_reg_readl(isp, OMAP3_ISP_IOMEM_CCP2, ISPCCP2_CTRL); diff --git a/drivers/media/platform/omap3isp/ispreg.h b/drivers/media/platform/omap3isp/ispreg.h index b5ea8da0b904..d08483919a77 100644 --- a/drivers/media/platform/omap3isp/ispreg.h +++ b/drivers/media/platform/omap3isp/ispreg.h @@ -87,6 +87,8 @@ #define ISPCCP2_CTRL_PHY_SEL_MASK 0x1 #define ISPCCP2_CTRL_PHY_SEL_SHIFT 1 #define ISPCCP2_CTRL_IO_OUT_SEL(1 << 2) +#define ISPCCP2_CTRL_IO_OUT_SEL_MASK 0x1 +#define ISPCCP2_CTRL_IO_OUT_SEL_SHIFT 2 #define ISPCCP2_CTRL_MODE (1 << 4) #define ISPCCP2_CTRL_VP_CLK_FORCE_ON (1 << 9) #define ISPCCP2_CTRL_INV (1 << 10) @@ -94,6 +96,8 @@ #define ISPCCP2_CTRL_INV_SHIFT 10 #define ISPCCP2_CTRL_VP_ONLY_EN(1 << 11) #define ISPCCP2_CTRL_VP_CLK_POL(1 << 12) +#define ISPCCP2_CTRL_VP_CLK_POL_MASK 0x1 +#define ISPCCP2_CTRL_VP_CLK_POL_SHIFT 12 #define ISPCCP2_CTRL_VPCLK_DIV_SHIFT 15 #define ISPCCP2_CTRL_VPCLK_DIV_MASK0x1 /* [31:15] */ #define ISPCCP2_CTRL_VP_OUT_CTRL_SHIFT 8 /* 3430 bits */ -- 2.11.0
[PATCH v2 4/5] omap3isp: csiphy: Don't assume the CSI receiver is a CSI2 module
The CSI PHY is associated with a CSI receiver. The code assumes this
receiver is a CSI2 module and relies on the CSI2 module object heavily to
access the ISP or pipeline objects. However, the receiver could also be a
CSI1/CCP2 module.
Pass a new CSI receiver entity pointer to the CSI PHY acquire function, and
replace all hardcoded usage of the CSI2 module with that CSI receiver
entity.
Signed-off-by: Sakari Ailus
Tested-by: Laurent Pinchart # on
Beagleboard-xM + MPT9P031
Acked-by: Pavel Machek
---
drivers/media/platform/omap3isp/ispccp2.c | 2 +-
drivers/media/platform/omap3isp/ispcsi2.c | 4 +--
drivers/media/platform/omap3isp/ispcsiphy.c | 45 +
drivers/media/platform/omap3isp/ispcsiphy.h | 6 ++--
4 files changed, 27 insertions(+), 30 deletions(-)
diff --git a/drivers/media/platform/omap3isp/ispccp2.c
b/drivers/media/platform/omap3isp/ispccp2.c
index 47210b102bcb..3db8df09cd9a 100644
--- a/drivers/media/platform/omap3isp/ispccp2.c
+++ b/drivers/media/platform/omap3isp/ispccp2.c
@@ -841,7 +841,7 @@ static int ccp2_s_stream(struct v4l2_subdev *sd, int enable)
switch (enable) {
case ISP_PIPELINE_STREAM_CONTINUOUS:
if (ccp2->phy) {
- ret = omap3isp_csiphy_acquire(ccp2->phy);
+ ret = omap3isp_csiphy_acquire(ccp2->phy, &sd->entity);
if (ret < 0)
return ret;
}
diff --git a/drivers/media/platform/omap3isp/ispcsi2.c
b/drivers/media/platform/omap3isp/ispcsi2.c
index 7dae2fe0d42d..3ec37fed710b 100644
--- a/drivers/media/platform/omap3isp/ispcsi2.c
+++ b/drivers/media/platform/omap3isp/ispcsi2.c
@@ -490,7 +490,7 @@ int omap3isp_csi2_reset(struct isp_csi2_device *csi2)
if (!csi2->available)
return -ENODEV;
- if (csi2->phy->phy_in_use)
+ if (csi2->phy->entity)
return -EBUSY;
isp_reg_set(isp, csi2->regs1, ISPCSI2_SYSCONFIG,
@@ -1053,7 +1053,7 @@ static int csi2_set_stream(struct v4l2_subdev *sd, int
enable)
switch (enable) {
case ISP_PIPELINE_STREAM_CONTINUOUS:
- if (omap3isp_csiphy_acquire(csi2->phy) < 0)
+ if (omap3isp_csiphy_acquire(csi2->phy, &sd->entity) < 0)
return -ENODEV;
if (csi2->output & CSI2_OUTPUT_MEMORY)
omap3isp_sbl_enable(isp, OMAP3_ISP_SBL_CSI2A_WRITE);
diff --git a/drivers/media/platform/omap3isp/ispcsiphy.c
b/drivers/media/platform/omap3isp/ispcsiphy.c
index 2028bb519108..ed1eb9907ae0 100644
--- a/drivers/media/platform/omap3isp/ispcsiphy.c
+++ b/drivers/media/platform/omap3isp/ispcsiphy.c
@@ -164,22 +164,17 @@ static int csiphy_set_power(struct isp_csiphy *phy, u32
power)
static int omap3isp_csiphy_config(struct isp_csiphy *phy)
{
- struct isp_csi2_device *csi2 = phy->csi2;
- struct isp_pipeline *pipe = to_isp_pipeline(&csi2->subdev.entity);
- struct isp_bus_cfg *buscfg = pipe->external->host_priv;
+ struct isp_pipeline *pipe = to_isp_pipeline(phy->entity);
+ struct isp_async_subdev *isd =
+ container_of(pipe->external->asd, struct isp_async_subdev, asd);
+ struct isp_bus_cfg *buscfg = pipe->external->host_priv ?
+ pipe->external->host_priv : &isd->bus;
struct isp_csiphy_lanes_cfg *lanes;
int csi2_ddrclk_khz;
unsigned int num_data_lanes, used_lanes = 0;
unsigned int i;
u32 reg;
- if (!buscfg) {
- struct isp_async_subdev *isd =
- container_of(pipe->external->asd,
-struct isp_async_subdev, asd);
- buscfg = &isd->bus;
- }
-
if (buscfg->interface == ISP_INTERFACE_CCP2B_PHY1
|| buscfg->interface == ISP_INTERFACE_CCP2B_PHY2) {
lanes = &buscfg->bus.ccp2.lanecfg;
@@ -222,7 +217,7 @@ static int omap3isp_csiphy_config(struct isp_csiphy *phy)
csi2_ddrclk_khz = pipe->external_rate / 1000
/ (2 * hweight32(used_lanes)) * pipe->external_width;
- reg = isp_reg_readl(csi2->isp, phy->phy_regs, ISPCSIPHY_REG0);
+ reg = isp_reg_readl(phy->isp, phy->phy_regs, ISPCSIPHY_REG0);
reg &= ~(ISPCSIPHY_REG0_THS_TERM_MASK |
ISPCSIPHY_REG0_THS_SETTLE_MASK);
@@ -233,9 +228,9 @@ static int omap3isp_csiphy_config(struct isp_csiphy *phy)
reg |= (DIV_ROUND_UP(90 * csi2_ddrclk_khz, 100) + 3)
<< ISPCSIPHY_REG0_THS_SETTLE_SHIFT;
- isp_reg_writel(csi2->isp, reg, phy->phy_regs, ISPCSIPHY_REG0);
+ isp_reg_writel(phy->isp, reg, phy->phy_regs, ISPCSIPHY_REG0);
- reg = isp_reg_readl(csi2->isp, phy->phy_regs, ISPCSIPHY_REG1);
+ reg = isp_reg_readl(phy->isp, phy->phy_regs, ISPCSIPHY_REG1);
reg &= ~(ISPCSIPHY_REG1_TCLK_TERM_MASK |
ISPCSIPHY_REG1_TCLK_MISS_MASK |
@@ -244,10 +239,10 @@ static int oma
[PATCH v2 1/5] omap3isp: Parse CSI1 configuration from the device tree
From: Pavel Machek
Add support for parsing CSI1 configuration.
Signed-off-by: Pavel Machek
Signed-off-by: Sakari Ailus
Reviewed-by: Laurent Pinchart
Reviewed-by: Sebastian Reichel
Tested-by: Laurent Pinchart # on
Beagleboard-xM + MPT9P031
---
drivers/media/platform/omap3isp/isp.c | 105 +
drivers/media/platform/omap3isp/omap3isp.h | 1 +
2 files changed, 79 insertions(+), 27 deletions(-)
diff --git a/drivers/media/platform/omap3isp/isp.c
b/drivers/media/platform/omap3isp/isp.c
index 79aff6b989a1..6cb1f0495804 100644
--- a/drivers/media/platform/omap3isp/isp.c
+++ b/drivers/media/platform/omap3isp/isp.c
@@ -2018,6 +2018,7 @@ static int isp_fwnode_parse(struct device *dev, struct
fwnode_handle *fwnode,
struct v4l2_fwnode_endpoint vep;
unsigned int i;
int ret;
+ bool csi1 = false;
ret = v4l2_fwnode_endpoint_parse(fwnode, &vep);
if (ret)
@@ -2047,41 +2048,91 @@ static int isp_fwnode_parse(struct device *dev, struct
fwnode_handle *fwnode,
case ISP_OF_PHY_CSIPHY1:
case ISP_OF_PHY_CSIPHY2:
- /* FIXME: always assume CSI-2 for now. */
+ switch (vep.bus_type) {
+ case V4L2_MBUS_CCP2:
+ case V4L2_MBUS_CSI1:
+ dev_dbg(dev, "CSI-1/CCP-2 configuration\n");
+ csi1 = true;
+ break;
+ case V4L2_MBUS_CSI2:
+ dev_dbg(dev, "CSI-2 configuration\n");
+ csi1 = false;
+ break;
+ default:
+ dev_err(dev, "unsupported bus type %u\n",
+ vep.bus_type);
+ return -EINVAL;
+ }
+
switch (vep.base.port) {
case ISP_OF_PHY_CSIPHY1:
- buscfg->interface = ISP_INTERFACE_CSI2C_PHY1;
+ if (csi1)
+ buscfg->interface = ISP_INTERFACE_CCP2B_PHY1;
+ else
+ buscfg->interface = ISP_INTERFACE_CSI2C_PHY1;
break;
case ISP_OF_PHY_CSIPHY2:
- buscfg->interface = ISP_INTERFACE_CSI2A_PHY2;
+ if (csi1)
+ buscfg->interface = ISP_INTERFACE_CCP2B_PHY2;
+ else
+ buscfg->interface = ISP_INTERFACE_CSI2A_PHY2;
break;
}
- buscfg->bus.csi2.lanecfg.clk.pos = vep.bus.mipi_csi2.clock_lane;
- buscfg->bus.csi2.lanecfg.clk.pol =
- vep.bus.mipi_csi2.lane_polarities[0];
- dev_dbg(dev, "clock lane polarity %u, pos %u\n",
- buscfg->bus.csi2.lanecfg.clk.pol,
- buscfg->bus.csi2.lanecfg.clk.pos);
-
- buscfg->bus.csi2.num_data_lanes =
- vep.bus.mipi_csi2.num_data_lanes;
-
- for (i = 0; i < buscfg->bus.csi2.num_data_lanes; i++) {
- buscfg->bus.csi2.lanecfg.data[i].pos =
- vep.bus.mipi_csi2.data_lanes[i];
- buscfg->bus.csi2.lanecfg.data[i].pol =
- vep.bus.mipi_csi2.lane_polarities[i + 1];
+ if (csi1) {
+ buscfg->bus.ccp2.lanecfg.clk.pos =
+ vep.bus.mipi_csi1.clock_lane;
+ buscfg->bus.ccp2.lanecfg.clk.pol =
+ vep.bus.mipi_csi1.lane_polarity[0];
+ dev_dbg(dev, "clock lane polarity %u, pos %u\n",
+ buscfg->bus.ccp2.lanecfg.clk.pol,
+ buscfg->bus.ccp2.lanecfg.clk.pos);
+
+ buscfg->bus.ccp2.lanecfg.data[0].pos =
+ vep.bus.mipi_csi1.data_lane;
+ buscfg->bus.ccp2.lanecfg.data[0].pol =
+ vep.bus.mipi_csi1.lane_polarity[1];
+
dev_dbg(dev, "data lane %u polarity %u, pos %u\n", i,
- buscfg->bus.csi2.lanecfg.data[i].pol,
- buscfg->bus.csi2.lanecfg.data[i].pos);
+ buscfg->bus.ccp2.lanecfg.data[0].pol,
+ buscfg->bus.ccp2.lanecfg.data[0].pos);
+
+ buscfg->bus.ccp2.strobe_clk_pol =
+ vep.bus.mipi_csi1.clock_inv;
+ buscfg->bus.ccp2.phy_layer = vep.bus.mipi_csi1.strobe;
+ buscfg->bus.ccp2.ccp2_mode =
+ vep.bus_type == V4L2_MBUS_CCP2;
+ buscfg->bus.ccp2.vp_clk_pol = 1;
+
+ buscfg->bus.ccp2.crc = 1;
+ } else {
+ buscfg->bus.csi2.lanecfg.clk
Re: [PATCH v2 4/5] omap3isp: csiphy: Don't assume the CSI receiver is a CSI2 module
Hi Sakari,
Thank you for the patch.
On Wednesday 16 Aug 2017 15:51:49 Sakari Ailus wrote:
> The CSI PHY is associated with a CSI receiver. The code assumes this
> receiver is a CSI2 module and relies on the CSI2 module object heavily to
> access the ISP or pipeline objects. However, the receiver could also be a
> CSI1/CCP2 module.
>
> Pass a new CSI receiver entity pointer to the CSI PHY acquire function, and
> replace all hardcoded usage of the CSI2 module with that CSI receiver
> entity.
>
> Signed-off-by: Sakari Ailus
> Tested-by: Laurent Pinchart # on
> Beagleboard-xM + MPT9P031
> Acked-by: Pavel Machek
Reviewed-by: Laurent Pinchart
> ---
> drivers/media/platform/omap3isp/ispccp2.c | 2 +-
> drivers/media/platform/omap3isp/ispcsi2.c | 4 +--
> drivers/media/platform/omap3isp/ispcsiphy.c | 45 +++--
> drivers/media/platform/omap3isp/ispcsiphy.h | 6 ++--
> 4 files changed, 27 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/media/platform/omap3isp/ispccp2.c
> b/drivers/media/platform/omap3isp/ispccp2.c index
> 47210b102bcb..3db8df09cd9a 100644
> --- a/drivers/media/platform/omap3isp/ispccp2.c
> +++ b/drivers/media/platform/omap3isp/ispccp2.c
> @@ -841,7 +841,7 @@ static int ccp2_s_stream(struct v4l2_subdev *sd, int
> enable) switch (enable) {
> case ISP_PIPELINE_STREAM_CONTINUOUS:
> if (ccp2->phy) {
> - ret = omap3isp_csiphy_acquire(ccp2->phy);
> + ret = omap3isp_csiphy_acquire(ccp2->phy, &sd->entity);
> if (ret < 0)
> return ret;
> }
> diff --git a/drivers/media/platform/omap3isp/ispcsi2.c
> b/drivers/media/platform/omap3isp/ispcsi2.c index
> 7dae2fe0d42d..3ec37fed710b 100644
> --- a/drivers/media/platform/omap3isp/ispcsi2.c
> +++ b/drivers/media/platform/omap3isp/ispcsi2.c
> @@ -490,7 +490,7 @@ int omap3isp_csi2_reset(struct isp_csi2_device *csi2)
> if (!csi2->available)
> return -ENODEV;
>
> - if (csi2->phy->phy_in_use)
> + if (csi2->phy->entity)
> return -EBUSY;
>
> isp_reg_set(isp, csi2->regs1, ISPCSI2_SYSCONFIG,
> @@ -1053,7 +1053,7 @@ static int csi2_set_stream(struct v4l2_subdev *sd, int
> enable)
>
> switch (enable) {
> case ISP_PIPELINE_STREAM_CONTINUOUS:
> - if (omap3isp_csiphy_acquire(csi2->phy) < 0)
> + if (omap3isp_csiphy_acquire(csi2->phy, &sd->entity) < 0)
> return -ENODEV;
> if (csi2->output & CSI2_OUTPUT_MEMORY)
> omap3isp_sbl_enable(isp, OMAP3_ISP_SBL_CSI2A_WRITE);
> diff --git a/drivers/media/platform/omap3isp/ispcsiphy.c
> b/drivers/media/platform/omap3isp/ispcsiphy.c index
> 2028bb519108..ed1eb9907ae0 100644
> --- a/drivers/media/platform/omap3isp/ispcsiphy.c
> +++ b/drivers/media/platform/omap3isp/ispcsiphy.c
> @@ -164,22 +164,17 @@ static int csiphy_set_power(struct isp_csiphy *phy,
> u32 power)
>
> static int omap3isp_csiphy_config(struct isp_csiphy *phy)
> {
> - struct isp_csi2_device *csi2 = phy->csi2;
> - struct isp_pipeline *pipe = to_isp_pipeline(&csi2->subdev.entity);
> - struct isp_bus_cfg *buscfg = pipe->external->host_priv;
> + struct isp_pipeline *pipe = to_isp_pipeline(phy->entity);
> + struct isp_async_subdev *isd =
> + container_of(pipe->external->asd, struct isp_async_subdev,
asd);
> + struct isp_bus_cfg *buscfg = pipe->external->host_priv ?
> + pipe->external->host_priv : &isd->bus;
> struct isp_csiphy_lanes_cfg *lanes;
> int csi2_ddrclk_khz;
> unsigned int num_data_lanes, used_lanes = 0;
> unsigned int i;
> u32 reg;
>
> - if (!buscfg) {
> - struct isp_async_subdev *isd =
> - container_of(pipe->external->asd,
> - struct isp_async_subdev, asd);
> - buscfg = &isd->bus;
> - }
> -
> if (buscfg->interface == ISP_INTERFACE_CCP2B_PHY1
>
> || buscfg->interface == ISP_INTERFACE_CCP2B_PHY2) {
>
> lanes = &buscfg->bus.ccp2.lanecfg;
> @@ -222,7 +217,7 @@ static int omap3isp_csiphy_config(struct isp_csiphy
> *phy) csi2_ddrclk_khz = pipe->external_rate / 1000
> / (2 * hweight32(used_lanes)) * pipe->external_width;
>
> - reg = isp_reg_readl(csi2->isp, phy->phy_regs, ISPCSIPHY_REG0);
> + reg = isp_reg_readl(phy->isp, phy->phy_regs, ISPCSIPHY_REG0);
>
> reg &= ~(ISPCSIPHY_REG0_THS_TERM_MASK |
>ISPCSIPHY_REG0_THS_SETTLE_MASK);
> @@ -233,9 +228,9 @@ static int omap3isp_csiphy_config(struct isp_csiphy
> *phy) reg |= (DIV_ROUND_UP(90 * csi2_ddrclk_khz, 100) + 3)
> << ISPCSIPHY_REG0_THS_SETTLE_SHIFT;
>
> - isp_reg_writel(csi2->isp, reg, phy->phy_regs, ISPCSIPHY_REG0);
> + isp_reg_writel(phy->isp, reg, phy->phy_regs, ISPCSIPHY_REG0);
>
> - reg = isp_reg_readl(csi2->isp, phy->phy_regs, ISPCSIPHY_REG1);
>
[PATCH 3/3] arm: dts: omap3: N9/N950: Add AS3645A camera flash
From: Sakari Ailus
Add the as3645a flash controller to the DT source as well as the flash
property with the as3645a device phandle to the sensor DT node.
Signed-off-by: Sakari Ailus
Reviewed-by: Sebastian Reichel
---
arch/arm/boot/dts/omap3-n9.dts | 1 +
arch/arm/boot/dts/omap3-n950-n9.dtsi | 14 ++
arch/arm/boot/dts/omap3-n950.dts | 1 +
3 files changed, 16 insertions(+)
diff --git a/arch/arm/boot/dts/omap3-n9.dts b/arch/arm/boot/dts/omap3-n9.dts
index b9e58c536afd..a2944010f62f 100644
--- a/arch/arm/boot/dts/omap3-n9.dts
+++ b/arch/arm/boot/dts/omap3-n9.dts
@@ -26,6 +26,7 @@
clocks = <&isp 0>;
clock-frequency = <960>;
nokia,nvm-size = <(16 * 64)>;
+ flash = <&as3645a_flash &as3645a_indicator>;
port {
smia_1_1: endpoint {
link-frequencies = /bits/ 64 <19920
21000 49920>;
diff --git a/arch/arm/boot/dts/omap3-n950-n9.dtsi
b/arch/arm/boot/dts/omap3-n950-n9.dtsi
index df3366fa5409..e15722b83a70 100644
--- a/arch/arm/boot/dts/omap3-n950-n9.dtsi
+++ b/arch/arm/boot/dts/omap3-n950-n9.dtsi
@@ -265,6 +265,20 @@
&i2c2 {
clock-frequency = <40>;
+
+ as3645a: flash@30 {
+ reg = <0x30>;
+ compatible = "ams,as3645a";
+ as3645a_flash: flash {
+ flash-timeout-us = <15>;
+ flash-max-microamp = <32>;
+ led-max-microamp = <6>;
+ peak-current-limit = <175>;
+ };
+ as3645a_indicator: indicator {
+ led-max-microamp = <1>;
+ };
+ };
};
&i2c3 {
diff --git a/arch/arm/boot/dts/omap3-n950.dts b/arch/arm/boot/dts/omap3-n950.dts
index 646601a3ebd8..bba5c5a6950c 100644
--- a/arch/arm/boot/dts/omap3-n950.dts
+++ b/arch/arm/boot/dts/omap3-n950.dts
@@ -60,6 +60,7 @@
clocks = <&isp 0>;
clock-frequency = <960>;
nokia,nvm-size = <(16 * 64)>;
+ flash = <&as3645a_flash &as3645a_indicator>;
port {
smia_1_1: endpoint {
link-frequencies = /bits/ 64 <21000
33360 39840>;
--
2.11.0
[PATCH 1/3] dt: bindings: Document DT bindings for Analog devices as3645a
From: Sakari Ailus
Signed-off-by: Sakari Ailus
---
.../devicetree/bindings/leds/ams,as3645a.txt | 56 ++
1 file changed, 56 insertions(+)
create mode 100644 Documentation/devicetree/bindings/leds/ams,as3645a.txt
diff --git a/Documentation/devicetree/bindings/leds/ams,as3645a.txt
b/Documentation/devicetree/bindings/leds/ams,as3645a.txt
new file mode 100644
index ..00066e3f9036
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/ams,as3645a.txt
@@ -0,0 +1,56 @@
+Analog devices AS3645A device tree bindings
+
+The AS3645A flash LED controller can drive two LEDs, one high current
+flash LED and one indicator LED. The high current flash LED can be
+used in torch mode as well.
+
+Ranges below noted as [a, b] are closed ranges between a and b, i.e. a
+and b are included in the range.
+
+
+Required properties
+===
+
+compatible : Must be "ams,as3645a".
+reg: The I2C address of the device. Typically 0x30.
+
+
+Required properties of the "flash" child node
+=
+
+flash-timeout-us: Flash timeout in microseconds. The value must be in
+ the range [10, 85] and divisible by 5.
+flash-max-microamp: Maximum flash current in microamperes. Has to be
+ in the range between [20, 50] and
+ divisible by 2.
+led-max-microamp: Maximum torch (assist) current in microamperes. The
+ value must be in the range between [2, 16] and
+ divisible by 2.
+ams,input-max-microamp: Maximum flash controller input current. The
+ value must be in the range [125, 200]
+ and divisible by 5.
+
+
+Required properties of the "indicator" child node
+=
+
+led-max-microamp: Maximum indicator current. The allowed values are
+ 2500, 5000, 7500 and 1.
+
+
+Example
+===
+
+ as3645a: flash@30 {
+ reg = <0x30>;
+ compatible = "ams,as3645a";
+ flash {
+ flash-timeout-us = <15>;
+ flash-max-microamp = <32>;
+ led-max-microamp = <6>;
+ ams,input-max-microamp = <175>;
+ };
+ indicator {
+ led-max-microamp = <1>;
+ };
+ };
--
2.11.0
[PATCH 2/3] leds: as3645a: Add LED flash class driver
From: Sakari Ailus Add a LED flash class driver for the as3654a flash controller. A V4L2 flash driver for it already exists (drivers/media/i2c/as3645a.c), and this driver is based on that. Signed-off-by: Sakari Ailus --- MAINTAINERS | 6 + drivers/leds/Kconfig| 8 + drivers/leds/Makefile | 1 + drivers/leds/leds-as3645a.c | 785 4 files changed, 800 insertions(+) create mode 100644 drivers/leds/leds-as3645a.c diff --git a/MAINTAINERS b/MAINTAINERS index 931abca006b7..8f40ba2e5303 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -2124,6 +2124,12 @@ F: arch/arm64/ F: Documentation/arm64/ AS3645A LED FLASH CONTROLLER DRIVER +M: Sakari Ailus +L: linux-l...@vger.kernel.org +S: Maintained +F: drivers/leds/leds-as3645a.c + +AS3645A LED FLASH CONTROLLER DRIVER M: Laurent Pinchart L: linux-media@vger.kernel.org T: git git://linuxtv.org/media_tree.git diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig index 594b24d410c3..bad3a4098104 100644 --- a/drivers/leds/Kconfig +++ b/drivers/leds/Kconfig @@ -58,6 +58,14 @@ config LEDS_AAT1290 help This option enables support for the LEDs on the AAT1290. +config LEDS_AS3645A + tristate "AS3645A LED flash controller support" + depends on I2C && LEDS_CLASS_FLASH + help + Enable LED flash class support for AS3645A LED flash + controller. V4L2 flash API is provided as well if + CONFIG_V4L2_FLASH_API is enabled. + config LEDS_BCM6328 tristate "LED Support for Broadcom BCM6328" depends on LEDS_CLASS diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile index 909dae62ba05..7d7b26552923 100644 --- a/drivers/leds/Makefile +++ b/drivers/leds/Makefile @@ -8,6 +8,7 @@ obj-$(CONFIG_LEDS_TRIGGERS) += led-triggers.o # LED Platform Drivers obj-$(CONFIG_LEDS_88PM860X)+= leds-88pm860x.o obj-$(CONFIG_LEDS_AAT1290) += leds-aat1290.o +obj-$(CONFIG_LEDS_AS3645A) += leds-as3645a.o obj-$(CONFIG_LEDS_BCM6328) += leds-bcm6328.o obj-$(CONFIG_LEDS_BCM6358) += leds-bcm6358.o obj-$(CONFIG_LEDS_BD2802) += leds-bd2802.o diff --git a/drivers/leds/leds-as3645a.c b/drivers/leds/leds-as3645a.c new file mode 100644 index ..2335510a08e1 --- /dev/null +++ b/drivers/leds/leds-as3645a.c @@ -0,0 +1,785 @@ +/* + * drivers/leds/leds-as3645a.c - AS3645A and LM3555 flash controllers driver + * + * Copyright (C) 2008-2011 Nokia Corporation + * Copyright (c) 2011, 2017 Intel Corporation. + * + * Based on drivers/media/i2c/as3645a.c. + * + * Contact: Sakari Ailus + * + * 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. + * + * This program is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include + +#define AS_TIMER_US_TO_CODE(t) (((t) / 1000 - 100) / 50) +#define AS_TIMER_CODE_TO_US(c) ((50 * (c) + 100) * 1000) + +/* Register definitions */ + +/* Read-only Design info register: Reset state: 0001 */ +#define AS_DESIGN_INFO_REG 0x00 +#define AS_DESIGN_INFO_FACTORY(x) (((x) >> 4)) +#define AS_DESIGN_INFO_MODEL(x)((x) & 0x0f) + +/* Read-only Version control register: Reset state: + * for first engineering samples + */ +#define AS_VERSION_CONTROL_REG 0x01 +#define AS_VERSION_CONTROL_RFU(x) (((x) >> 4)) +#define AS_VERSION_CONTROL_VERSION(x) ((x) & 0x0f) + +/* Read / Write(Indicator and timer register): Reset state: */ +#define AS_INDICATOR_AND_TIMER_REG 0x02 +#define AS_INDICATOR_AND_TIMER_TIMEOUT_SHIFT 0 +#define AS_INDICATOR_AND_TIMER_VREF_SHIFT 4 +#define AS_INDICATOR_AND_TIMER_INDICATOR_SHIFT 6 + +/* Read / Write(Current set register): Reset state: 0110 1001 */ +#define AS_CURRENT_SET_REG 0x03 +#define AS_CURRENT_ASSIST_LIGHT_SHIFT 0 +#define AS_CURRENT_LED_DET_ON (1 << 3) +#define AS_CURRENT_FLASH_CURRENT_SHIFT 4 + +/* Read / Write(Control register): Reset state: 1011 0100 */ +#define AS_CONTROL_REG 0x04 +#define AS_CONTROL_MODE_SETTING_SHIFT 0 +#define AS_CONTROL_STROBE_ON (1 << 2) +#define AS_CONTROL_OUT_ON (1 << 3) +#define AS_CONTROL_EXT_TORCH_ON(1 << 4) +#define AS_CONTROL_STROBE_TYPE_EDGE(0 << 5) +#define AS_CONTROL
[PATCH 0/3] AS3645A flash support
Hi everyone,
This set adds support for the AS3645A flash driver which can be found e.g.
in Nokia N9.
The set depeds on the flash patches here so I'd prefer to merge this
through mediatree.
https://git.linuxtv.org/sailus/media_tree.git/log/?h=flash>
Jacek: would that be ok for you?
Since the RFC set:
- Add back the DT binding documentation I lost long ago.
- Use colon (":") in the default names instead of a space.
Sakari Ailus (3):
dt: bindings: Document DT bindings for Analog devices as3645a
leds: as3645a: Add LED flash class driver
arm: dts: omap3: N9/N950: Add AS3645A camera flash
.../devicetree/bindings/leds/ams,as3645a.txt | 56 ++
MAINTAINERS| 6 +
arch/arm/boot/dts/omap3-n9.dts | 1 +
arch/arm/boot/dts/omap3-n950-n9.dtsi | 14 +
arch/arm/boot/dts/omap3-n950.dts | 1 +
drivers/leds/Kconfig | 8 +
drivers/leds/Makefile | 1 +
drivers/leds/leds-as3645a.c| 785 +
8 files changed, 872 insertions(+)
create mode 100644 Documentation/devicetree/bindings/leds/ams,as3645a.txt
create mode 100644 drivers/leds/leds-as3645a.c
--
2.11.0
Re: [PATCH 2/3] leds: as3645a: Add LED flash class driver
Hello Sakari,
I haven't looked at the driver, but just have a comment about the I2C subsystem.
On Wed, Aug 16, 2017 at 2:55 PM, Sakari Ailus
wrote:
[snip]
> +
> +static const struct of_device_id as3645a_of_table[] = {
> + { .compatible = "ams,as3645a" },
> + { },
> +};
> +MODULE_DEVICE_TABLE(of, as3645a_of_table);
> +
> +SIMPLE_DEV_PM_OPS(as3645a_pm_ops, as3645a_resume, as3645a_suspend);
> +
> +static struct i2c_driver as3645a_i2c_driver = {
> + .driver = {
> + .of_match_table = as3645a_of_table,
> + .name = AS_NAME,
> + .pm = &as3645a_pm_ops,
> + },
> + .probe_new = as3645a_probe,
> + .remove = as3645a_remove,
> +};
> +
> +module_i2c_driver(as3645a_i2c_driver);
> +
The I2C core is still broken w.r.t reporting a proper MODALIAS for OF
registered devices, it will report a MODALIAS=i2c:as3645a in this
case. So if you build this as a module, autoload won't work.
In theory this will be fixed soon, but for now you should add a
i2c_device_id table and export it with MODULE_DEVICE_TABLE(i2c,...) if
you care about module autoloading.
Best regards,
Javier
Re: [PATCH v3 1/4] i2c: add helpers to ease DMA handling
Hi Wolfram,
On Tue, Jul 18, 2017 at 12:23 PM, Wolfram Sang
wrote:
> One helper checks if DMA is suitable and optionally creates a bounce
> buffer, if not. The other function returns the bounce buffer and makes
> sure the data is properly copied back to the message.
>
> Signed-off-by: Wolfram Sang
> ---
> Changes since v2:
>
> * rebased to v4.13-rc1
> * helper functions are not inlined anymore but moved to i2c core
> * __must_check has been added to the buffer check helper
> * the release function has been renamed to contain 'dma' as well
Right:
drivers/i2c/i2c-core-base.c:2310:15: error: 'i2c_release_bounce_buf'
undeclared here (not in a function)
EXPORT_SYMBOL_GPL(i2c_release_bounce_buf);
> --- a/drivers/i2c/i2c-core-base.c
> +++ b/drivers/i2c/i2c-core-base.c
> +/**
> + * i2c_release_bounce_buf - copy data back from bounce buffer and release it
^^
> + * @msg: the message to be copied back to
> + * @bounce_buf: the bounce buffer obtained from i2c_check_msg_for_dma().
> + * May be NULL.
> + */
> +void i2c_release_dma_bounce_buf(struct i2c_msg *msg, u8 *bounce_buf)
> +{
> + if (!bounce_buf)
> + return;
> +
> + if (msg->flags & I2C_M_RD)
> + memcpy(msg->buf, bounce_buf, msg->len);
> +
> + kfree(bounce_buf);
> +}
> +EXPORT_SYMBOL_GPL(i2c_release_bounce_buf);
^^
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
[PATCH] [media] cx18: Fix incompatible type for argument error
The first argument of function snd_ctl_new1 is of type const struct
snd_kcontrol_new * but in this file the variable is passed by value to
this function.
This generated ""incompatible type for argument" error. So, pass the
variable by reference.
Signed-off-by: Bhumika Goyal
---
I was not able to compile this file and I could not find any reference to
this file in any Makefile. Also, I could not find any reference to
anything defined in this file. That means the code is not used anywhere.
So, should I send a patch for removing the file?
drivers/media/pci/cx18/cx18-alsa-mixer.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/media/pci/cx18/cx18-alsa-mixer.c
b/drivers/media/pci/cx18/cx18-alsa-mixer.c
index 06b066b..cb04c3d 100644
--- a/drivers/media/pci/cx18/cx18-alsa-mixer.c
+++ b/drivers/media/pci/cx18/cx18-alsa-mixer.c
@@ -161,7 +161,7 @@ int __init snd_cx18_mixer_create(struct snd_cx18_card *cxsc)
strlcpy(sc->mixername, "CX23418 Mixer", sizeof(sc->mixername));
- ret = snd_ctl_add(sc, snd_ctl_new1(snd_cx18_mixer_tv_vol, cxsc));
+ ret = snd_ctl_add(sc, snd_ctl_new1(&snd_cx18_mixer_tv_vol, cxsc));
if (ret) {
CX18_ALSA_WARN("%s: failed to add %s control, err %d\n",
__func__, snd_cx18_mixer_tv_vol.name, ret);
--
1.9.1
Re: [PATCH] uvcvideo: Apply flags from device to actual properties
Hi Edgar,
Thank you for the patch.
On Tuesday 15 Aug 2017 12:59:47 Edgar Thier wrote:
> Use flags the device exposes for UVC controls.
In addition to explaining what the patch does, the commit message should
explain why the change is needed. What is the problem you're trying to address
here ?
> Signed-off-by: Edgar Thier
> ---
> drivers/media/usb/uvc/uvc_ctrl.c | 26 +-
> 1 file changed, 25 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/usb/uvc/uvc_ctrl.c
> b/drivers/media/usb/uvc/uvc_ctrl.c index c2ee6e3..bc69e92 100644
> --- a/drivers/media/usb/uvc/uvc_ctrl.c
> +++ b/drivers/media/usb/uvc/uvc_ctrl.c
> @@ -1568,7 +1568,8 @@ int uvc_ctrl_set(struct uvc_video_chain *chain,
> return ret;
> }
>
> - ctrl->loaded = 1;
> + if (!(ctrl->info.flags && UVC_CTRL_FLAG_AUTO_UPDATE))
> + ctrl->loaded = 1;
I think this change is unrelated to the subject of this patch. It should be
split to a separate patch.
This being said, why is this change needed ? The uvc_ctrl_set() function is
called from uvc_ctrl_s_ctrl() and uvc_ioctl_s_try_ext_ctrls(), and followed by
a uvc_ctrl_rollback() or uvc_ctrl_commit() call that will set the loaded flag
back to 0 in uvc_ctrl_commit_entity().
> }
>
> /* Backup the current value in case we need to rollback later. */
> @@ -1889,8 +1890,13 @@ int uvc_ctrl_restore_values(struct uvc_device *dev)
> static int uvc_ctrl_add_info(struct uvc_device *dev, struct uvc_control
> *ctrl, const struct uvc_control_info *info)
> {
> + u8 *data;
> int ret = 0;
>
> + data = kmalloc(2, GFP_KERNEL);
> + if (data == NULL)
> + return -ENOMEM;
> +
> ctrl->info = *info;
> INIT_LIST_HEAD(&ctrl->info.mappings);
>
> @@ -1904,6 +1910,23 @@ static int uvc_ctrl_add_info(struct uvc_device *dev,
> struct uvc_control *ctrl,
>
> ctrl->initialized = 1;
>
> + ret = uvc_query_ctrl(dev, UVC_GET_INFO, ctrl->entity->id, dev-
>intfnum,
> + info->selector, data, 1);
> + if (ret < 0) {
> + uvc_trace(UVC_TRACE_CONTROL,
> + "GET_INFO failed on control %pUl/%u (%d).\n",
> + info->entity, info->selector, ret);
> + }
> + else {
> + ctrl->info.flags = UVC_CTRL_FLAG_GET_MIN |
UVC_CTRL_FLAG_GET_MAX
> + | UVC_CTRL_FLAG_GET_RES | UVC_CTRL_FLAG_GET_DEF
> + | (data[0] & UVC_CONTROL_CAP_GET ?
> +UVC_CTRL_FLAG_GET_CUR : 0)
> + | (data[0] & UVC_CONTROL_CAP_SET ?
> +UVC_CTRL_FLAG_SET_CUR : 0)
> + | (data[0] & UVC_CONTROL_CAP_AUTOUPDATE ?
> +UVC_CTRL_FLAG_AUTO_UPDATE : 0);
> + }
This code seems copied from uvc_ctrl_fill_xu_info(). I'd rather move it to a
function that can be called by both instead of duplicating it.
> uvc_trace(UVC_TRACE_CONTROL, "Added control %pUl/%u to device %s "
> "entity %u\n", ctrl->info.entity, ctrl->info.selector,
> dev->udev->devpath, ctrl->entity->id);
> @@ -1911,6 +1934,7 @@ static int uvc_ctrl_add_info(struct uvc_device *dev,
> struct uvc_control *ctrl, done:
> if (ret < 0)
> kfree(ctrl->uvc_data);
> + kfree(data);
> return ret;
> }
--
Regards,
Laurent Pinchart
[PATCH] [media] ivtv: Fix incompatible type for argument error
The first argument of function snd_ctl_new1 is of type const struct
snd_kcontrol_new * but in this file the variable is passed by value
to this function. This generated ""incompatible type for argument"
error. So, pass the variable by reference.
Signed-off-by: Bhumika Goyal
---
I was not able to compile this file and I could not find any reference to
this file in any Makefile. Also, I could not find any reference to
anything defined in this file. That means the code is not used anywhere.
So, should I send a patch for removing the file?
drivers/media/pci/ivtv/ivtv-alsa-mixer.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/media/pci/ivtv/ivtv-alsa-mixer.c
b/drivers/media/pci/ivtv/ivtv-alsa-mixer.c
index ba372a2..aee453f 100644
--- a/drivers/media/pci/ivtv/ivtv-alsa-mixer.c
+++ b/drivers/media/pci/ivtv/ivtv-alsa-mixer.c
@@ -156,7 +156,7 @@ int __init snd_ivtv_mixer_create(struct snd_ivtv_card
*itvsc)
strlcpy(sc->mixername, "CX2341[56] Mixer", sizeof(sc->mixername));
- ret = snd_ctl_add(sc, snd_ctl_new1(snd_ivtv_mixer_tv_vol, itvsc));
+ ret = snd_ctl_add(sc, snd_ctl_new1(&snd_ivtv_mixer_tv_vol, itvsc));
if (ret) {
IVTV_ALSA_WARN("%s: failed to add %s control, err %d\n",
__func__, snd_ivtv_mixer_tv_vol.name, ret);
--
1.9.1
Re: [PATCH v3 1/4] i2c: add helpers to ease DMA handling
> Right: > > drivers/i2c/i2c-core-base.c:2310:15: error: 'i2c_release_bounce_buf' > undeclared here (not in a function) > EXPORT_SYMBOL_GPL(i2c_release_bounce_buf); Thanks. I am just now working on V4 currently which is a redesign. I'll write more in an hour or so. signature.asc Description: PGP signature
[PATCH v2.1 5/5] omap3isp: Quit using struct v4l2_subdev.host_priv field
struct v4l2_subdev.host_priv is intended to be used by another driver. This
is hardly good design but back in the days of platform data was a quick
hack to get things done.
As the sub-device specific bus information can be stored to the ISP driver
specific struct allocated along with v4l2_async_subdev, keep the
information there and only there.
Signed-off-by: Sakari Ailus
---
This patch replaces patch "omap3isp: Correctly configure routing in PHY
release".
drivers/media/platform/omap3isp/isp.c | 31 +
drivers/media/platform/omap3isp/ispccdc.c | 20 ---
drivers/media/platform/omap3isp/ispccp2.c | 4 +++-
drivers/media/platform/omap3isp/ispcsi2.c | 3 ++-
drivers/media/platform/omap3isp/ispcsiphy.c | 11 +-
5 files changed, 33 insertions(+), 36 deletions(-)
diff --git a/drivers/media/platform/omap3isp/isp.c
b/drivers/media/platform/omap3isp/isp.c
index 6cb1f0495804..27c577fac8e9 100644
--- a/drivers/media/platform/omap3isp/isp.c
+++ b/drivers/media/platform/omap3isp/isp.c
@@ -2188,26 +2188,12 @@ static int isp_fwnodes_parse(struct device *dev,
return -EINVAL;
}
-static int isp_subdev_notifier_bound(struct v4l2_async_notifier *async,
-struct v4l2_subdev *subdev,
-struct v4l2_async_subdev *asd)
-{
- struct isp_async_subdev *isd =
- container_of(asd, struct isp_async_subdev, asd);
-
- isd->sd = subdev;
- isd->sd->host_priv = &isd->bus;
-
- return 0;
-}
-
static int isp_subdev_notifier_complete(struct v4l2_async_notifier *async)
{
struct isp_device *isp = container_of(async, struct isp_device,
notifier);
struct v4l2_device *v4l2_dev = &isp->v4l2_dev;
struct v4l2_subdev *sd;
- struct isp_bus_cfg *bus;
int ret;
ret = media_entity_enum_init(&isp->crashed, &isp->media_dev);
@@ -2215,13 +2201,15 @@ static int isp_subdev_notifier_complete(struct
v4l2_async_notifier *async)
return ret;
list_for_each_entry(sd, &v4l2_dev->subdevs, list) {
- /* Only try to link entities whose interface was set on bound */
- if (sd->host_priv) {
- bus = (struct isp_bus_cfg *)sd->host_priv;
- ret = isp_link_entity(isp, &sd->entity, bus->interface);
- if (ret < 0)
- return ret;
- }
+ struct isp_async_subdev *isd;
+
+ if (!sd->asd)
+ continue;
+
+ isd = container_of(sd->asd, struct isp_async_subdev, asd);
+ ret = isp_link_entity(isp, &sd->entity, isd->bus.interface);
+ if (ret < 0)
+ return ret;
}
ret = v4l2_device_register_subdev_nodes(&isp->v4l2_dev);
@@ -2399,7 +2387,6 @@ static int isp_probe(struct platform_device *pdev)
if (ret < 0)
goto error_register_entities;
- isp->notifier.bound = isp_subdev_notifier_bound;
isp->notifier.complete = isp_subdev_notifier_complete;
ret = v4l2_async_notifier_register(&isp->v4l2_dev, &isp->notifier);
diff --git a/drivers/media/platform/omap3isp/ispccdc.c
b/drivers/media/platform/omap3isp/ispccdc.c
index 4947876cfadf..0145b3dcd7a7 100644
--- a/drivers/media/platform/omap3isp/ispccdc.c
+++ b/drivers/media/platform/omap3isp/ispccdc.c
@@ -1139,8 +1139,12 @@ static void ccdc_configure(struct isp_ccdc_device *ccdc)
pad = media_entity_remote_pad(&ccdc->pads[CCDC_PAD_SINK]);
sensor = media_entity_to_v4l2_subdev(pad->entity);
if (ccdc->input == CCDC_INPUT_PARALLEL) {
- parcfg = &((struct isp_bus_cfg *)sensor->host_priv)
- ->bus.parallel;
+ struct isp_pipeline *pipe =
+ to_isp_pipeline(&ccdc->subdev.entity);
+
+ parcfg = &container_of(pipe->external->asd,
+ struct isp_async_subdev,
+ asd)->bus.bus.parallel;
ccdc->bt656 = parcfg->bt656;
}
@@ -2412,11 +2416,13 @@ static int ccdc_link_validate(struct v4l2_subdev *sd,
/* We've got a parallel sensor here. */
if (ccdc->input == CCDC_INPUT_PARALLEL) {
- struct isp_parallel_cfg *parcfg =
- &((struct isp_bus_cfg *)
- media_entity_to_v4l2_subdev(link->source->entity)
- ->host_priv)->bus.parallel;
- parallel_shift = parcfg->data_lane_shift;
+ struct isp_pipeline *pipe =
+ to_isp_pipeline(&ccdc->subdev.entity);
+
+ parallel_shift =
+ container_of(pipe->external->asd,
+struct isp_async_subdev,
+
Re: [PATCH] keytable: ensure udev rule fires on rc input device
On Mon, Aug 07, 2017 at 09:09:26AM +0200, Matthias Reichl wrote:
> Hi Sean!
>
> On Sun, Aug 06, 2017 at 09:56:55AM +0100, Sean Young wrote:
> > The rc device is created before the input device, so if ir-keytable runs
> > too early the input device does not exist yet.
> >
> > Ensure that rule fires on creation of a rc device's input device.
> >
> > Note that this also prevents udev from starting ir-keytable on an
> > transmit only device, which has no input device.
> >
> > Signed-off-by: Sean Young
>
> Signed-off-by: Matthias Reichl
>
> One comment though, see below
>
> > ---
> > utils/keytable/70-infrared.rules | 10 +-
> > 1 file changed, 9 insertions(+), 1 deletion(-)
> >
> > Matthias, can I have your Signed-off-by please? Thank you.
> >
> >
> > diff --git a/utils/keytable/70-infrared.rules
> > b/utils/keytable/70-infrared.rules
> > index afffd951..b3531727 100644
> > --- a/utils/keytable/70-infrared.rules
> > +++ b/utils/keytable/70-infrared.rules
> > @@ -1,4 +1,12 @@
> > # Automatically load the proper keymaps after the Remote Controller device
> > # creation. The keycode tables rules should be at /etc/rc_maps.cfg
> >
> > -ACTION=="add", SUBSYSTEM=="rc", RUN+="/usr/bin/ir-keytable -a
> > /etc/rc_maps.cfg -s $name"
> > +ACTION!="add", SUBSYSTEMS!="rc", GOTO="rc_dev_end"
>
> This line doesn't quite what we want it to do.
>
> As SUBSYSTEMS!="rc" is basically a no-op and would only be
> evaluated on change/remove events anyways that line boils down to
>
> ACTION!="add", GOTO="rc_dev_end"
>
> and the following rules are evaluated on all add events.
Yes, you're right. The goto is only executed if all the preceeding matches,
and for ACTION=add that is never the case.
> While that'll still work it'll do unnecessary work, like importing
> rc_sydev for all input devices and could bite us (or users) later
> if we change/extend the ruleset.
>
> Better do it like in my original comment (using positive logic and
> a GOTO="begin") or use ACTION!="add", GOTO="rc_dev_end" and add
> SUBSYSTEMS=="rc" to the IMPORT and RUN rules below.
I've found a shorter way of doing this.
Sean
From: Sean Young
Date: Wed, 16 Aug 2017 17:41:53 +0100
Subject: [PATCH] keytable: ensure the udev rule fires on creation of the input
device
The rc device is created before the input device, so if ir-keytable runs
too early the input device does not exist yet.
Ensure that rule fires on creation of a rc device's input device.
Note that this also prevents udev from starting ir-keytable on an
transmit only device, which has no input device.
Note that $id in RUN will not work, since that is expanded after all the
rules are matched, at which point the the parent might have been changed
by another match in another rule. The argument to $env{key} is expanded
immediately.
Signed-off-by: Sean Young
---
utils/keytable/70-infrared.rules | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/utils/keytable/70-infrared.rules b/utils/keytable/70-infrared.rules
index afffd951..41ca2089 100644
--- a/utils/keytable/70-infrared.rules
+++ b/utils/keytable/70-infrared.rules
@@ -1,4 +1,4 @@
# Automatically load the proper keymaps after the Remote Controller device
# creation. The keycode tables rules should be at /etc/rc_maps.cfg
-ACTION=="add", SUBSYSTEM=="rc", RUN+="/usr/bin/ir-keytable -a /etc/rc_maps.cfg
-s $name"
+ACTION=="add", SUBSYSTEM=="input", SUBSYSTEMS=="rc", KERNEL=="event*",
ENV{.rc_sysdev}="$id", RUN+="/usr/bin/ir-keytable -a /etc/rc_maps.cfg -s
$env{.rc_sysdev}"
--
2.13.5
[PATCH v2.2 5/5] omap3isp: Quit using struct v4l2_subdev.host_priv field
struct v4l2_subdev.host_priv is intended to be used by another driver. This
is hardly good design but back in the days of platform data was a quick
hack to get things done.
As the sub-device specific bus information can be stored to the ISP driver
specific struct allocated along with v4l2_async_subdev, keep the
information there and only there.
Signed-off-by: Sakari Ailus
---
since v2.1:
- Remove struct isp_async_subdev.sd field as it is now redundant.
drivers/media/platform/omap3isp/isp.c | 31 +
drivers/media/platform/omap3isp/isp.h | 1 -
drivers/media/platform/omap3isp/ispccdc.c | 20 ---
drivers/media/platform/omap3isp/ispccp2.c | 4 +++-
drivers/media/platform/omap3isp/ispcsi2.c | 3 ++-
drivers/media/platform/omap3isp/ispcsiphy.c | 11 +-
6 files changed, 33 insertions(+), 37 deletions(-)
diff --git a/drivers/media/platform/omap3isp/isp.c
b/drivers/media/platform/omap3isp/isp.c
index 6cb1f0495804..27c577fac8e9 100644
--- a/drivers/media/platform/omap3isp/isp.c
+++ b/drivers/media/platform/omap3isp/isp.c
@@ -2188,26 +2188,12 @@ static int isp_fwnodes_parse(struct device *dev,
return -EINVAL;
}
-static int isp_subdev_notifier_bound(struct v4l2_async_notifier *async,
-struct v4l2_subdev *subdev,
-struct v4l2_async_subdev *asd)
-{
- struct isp_async_subdev *isd =
- container_of(asd, struct isp_async_subdev, asd);
-
- isd->sd = subdev;
- isd->sd->host_priv = &isd->bus;
-
- return 0;
-}
-
static int isp_subdev_notifier_complete(struct v4l2_async_notifier *async)
{
struct isp_device *isp = container_of(async, struct isp_device,
notifier);
struct v4l2_device *v4l2_dev = &isp->v4l2_dev;
struct v4l2_subdev *sd;
- struct isp_bus_cfg *bus;
int ret;
ret = media_entity_enum_init(&isp->crashed, &isp->media_dev);
@@ -2215,13 +2201,15 @@ static int isp_subdev_notifier_complete(struct
v4l2_async_notifier *async)
return ret;
list_for_each_entry(sd, &v4l2_dev->subdevs, list) {
- /* Only try to link entities whose interface was set on bound */
- if (sd->host_priv) {
- bus = (struct isp_bus_cfg *)sd->host_priv;
- ret = isp_link_entity(isp, &sd->entity, bus->interface);
- if (ret < 0)
- return ret;
- }
+ struct isp_async_subdev *isd;
+
+ if (!sd->asd)
+ continue;
+
+ isd = container_of(sd->asd, struct isp_async_subdev, asd);
+ ret = isp_link_entity(isp, &sd->entity, isd->bus.interface);
+ if (ret < 0)
+ return ret;
}
ret = v4l2_device_register_subdev_nodes(&isp->v4l2_dev);
@@ -2399,7 +2387,6 @@ static int isp_probe(struct platform_device *pdev)
if (ret < 0)
goto error_register_entities;
- isp->notifier.bound = isp_subdev_notifier_bound;
isp->notifier.complete = isp_subdev_notifier_complete;
ret = v4l2_async_notifier_register(&isp->v4l2_dev, &isp->notifier);
diff --git a/drivers/media/platform/omap3isp/isp.h
b/drivers/media/platform/omap3isp/isp.h
index 2f2ae609c548..25472c81dcdd 100644
--- a/drivers/media/platform/omap3isp/isp.h
+++ b/drivers/media/platform/omap3isp/isp.h
@@ -226,7 +226,6 @@ struct isp_device {
};
struct isp_async_subdev {
- struct v4l2_subdev *sd;
struct isp_bus_cfg bus;
struct v4l2_async_subdev asd;
};
diff --git a/drivers/media/platform/omap3isp/ispccdc.c
b/drivers/media/platform/omap3isp/ispccdc.c
index 4947876cfadf..0145b3dcd7a7 100644
--- a/drivers/media/platform/omap3isp/ispccdc.c
+++ b/drivers/media/platform/omap3isp/ispccdc.c
@@ -1139,8 +1139,12 @@ static void ccdc_configure(struct isp_ccdc_device *ccdc)
pad = media_entity_remote_pad(&ccdc->pads[CCDC_PAD_SINK]);
sensor = media_entity_to_v4l2_subdev(pad->entity);
if (ccdc->input == CCDC_INPUT_PARALLEL) {
- parcfg = &((struct isp_bus_cfg *)sensor->host_priv)
- ->bus.parallel;
+ struct isp_pipeline *pipe =
+ to_isp_pipeline(&ccdc->subdev.entity);
+
+ parcfg = &container_of(pipe->external->asd,
+ struct isp_async_subdev,
+ asd)->bus.bus.parallel;
ccdc->bt656 = parcfg->bt656;
}
@@ -2412,11 +2416,13 @@ static int ccdc_link_validate(struct v4l2_subdev *sd,
/* We've got a parallel sensor here. */
if (ccdc->input == CCDC_INPUT_PARALLEL) {
- struct isp_parallel_cfg *parcfg =
- &((struct isp_bus_cfg *)
- media_ent
Re: [PATCH v2.2 5/5] omap3isp: Quit using struct v4l2_subdev.host_priv field
Hi Sakari,
Thank you for the patch.
On Wednesday 16 Aug 2017 20:05:52 Sakari Ailus wrote:
> struct v4l2_subdev.host_priv is intended to be used by another driver. This
> is hardly good design but back in the days of platform data was a quick
> hack to get things done.
>
> As the sub-device specific bus information can be stored to the ISP driver
> specific struct allocated along with v4l2_async_subdev, keep the
> information there and only there.
>
> Signed-off-by: Sakari Ailus
This looks better to me than the previous approach. Please see below for one
last comment.
> ---
> since v2.1:
>
> - Remove struct isp_async_subdev.sd field as it is now redundant.
>
> drivers/media/platform/omap3isp/isp.c | 31 --
> drivers/media/platform/omap3isp/isp.h | 1 -
> drivers/media/platform/omap3isp/ispccdc.c | 20 ---
> drivers/media/platform/omap3isp/ispccp2.c | 4 +++-
> drivers/media/platform/omap3isp/ispcsi2.c | 3 ++-
> drivers/media/platform/omap3isp/ispcsiphy.c | 11 +-
> 6 files changed, 33 insertions(+), 37 deletions(-)
>
> diff --git a/drivers/media/platform/omap3isp/isp.c
> b/drivers/media/platform/omap3isp/isp.c index 6cb1f0495804..27c577fac8e9
> 100644
> --- a/drivers/media/platform/omap3isp/isp.c
> +++ b/drivers/media/platform/omap3isp/isp.c
> @@ -2188,26 +2188,12 @@ static int isp_fwnodes_parse(struct device *dev,
> return -EINVAL;
> }
>
> -static int isp_subdev_notifier_bound(struct v4l2_async_notifier *async,
> - struct v4l2_subdev *subdev,
> - struct v4l2_async_subdev *asd)
> -{
> - struct isp_async_subdev *isd =
> - container_of(asd, struct isp_async_subdev, asd);
> -
> - isd->sd = subdev;
> - isd->sd->host_priv = &isd->bus;
> -
> - return 0;
> -}
> -
> static int isp_subdev_notifier_complete(struct v4l2_async_notifier *async)
> {
> struct isp_device *isp = container_of(async, struct isp_device,
> notifier);
> struct v4l2_device *v4l2_dev = &isp->v4l2_dev;
> struct v4l2_subdev *sd;
> - struct isp_bus_cfg *bus;
> int ret;
>
> ret = media_entity_enum_init(&isp->crashed, &isp->media_dev);
> @@ -2215,13 +2201,15 @@ static int isp_subdev_notifier_complete(struct
> v4l2_async_notifier *async) return ret;
>
> list_for_each_entry(sd, &v4l2_dev->subdevs, list) {
> - /* Only try to link entities whose interface was set on bound
*/
> - if (sd->host_priv) {
> - bus = (struct isp_bus_cfg *)sd->host_priv;
> - ret = isp_link_entity(isp, &sd->entity, bus-
>interface);
> - if (ret < 0)
> - return ret;
> - }
> + struct isp_async_subdev *isd;
> +
> + if (!sd->asd)
> + continue;
> +
> + isd = container_of(sd->asd, struct isp_async_subdev, asd);
> + ret = isp_link_entity(isp, &sd->entity, isd->bus.interface);
> + if (ret < 0)
> + return ret;
> }
>
> ret = v4l2_device_register_subdev_nodes(&isp->v4l2_dev);
> @@ -2399,7 +2387,6 @@ static int isp_probe(struct platform_device *pdev)
> if (ret < 0)
> goto error_register_entities;
>
> - isp->notifier.bound = isp_subdev_notifier_bound;
> isp->notifier.complete = isp_subdev_notifier_complete;
>
> ret = v4l2_async_notifier_register(&isp->v4l2_dev, &isp->notifier);
> diff --git a/drivers/media/platform/omap3isp/isp.h
> b/drivers/media/platform/omap3isp/isp.h index 2f2ae609c548..25472c81dcdd
> 100644
> --- a/drivers/media/platform/omap3isp/isp.h
> +++ b/drivers/media/platform/omap3isp/isp.h
> @@ -226,7 +226,6 @@ struct isp_device {
> };
>
> struct isp_async_subdev {
> - struct v4l2_subdev *sd;
> struct isp_bus_cfg bus;
> struct v4l2_async_subdev asd;
> };
> diff --git a/drivers/media/platform/omap3isp/ispccdc.c
> b/drivers/media/platform/omap3isp/ispccdc.c index
> 4947876cfadf..0145b3dcd7a7 100644
> --- a/drivers/media/platform/omap3isp/ispccdc.c
> +++ b/drivers/media/platform/omap3isp/ispccdc.c
> @@ -1139,8 +1139,12 @@ static void ccdc_configure(struct isp_ccdc_device
> *ccdc) pad = media_entity_remote_pad(&ccdc->pads[CCDC_PAD_SINK]);
> sensor = media_entity_to_v4l2_subdev(pad->entity);
> if (ccdc->input == CCDC_INPUT_PARALLEL) {
> - parcfg = &((struct isp_bus_cfg *)sensor->host_priv)
> - ->bus.parallel;
> + struct isp_pipeline *pipe =
> + to_isp_pipeline(&ccdc->subdev.entity);
> +
> + parcfg = &container_of(pipe->external->asd,
> +struct isp_async_subdev,
> +asd)->bus.bus.parallel;
You use this construct in many places, how about adding
struct isp_bus_cfg *to_bus_cf
Re: [PATCH v2.2 5/5] omap3isp: Quit using struct v4l2_subdev.host_priv field
On Wed, Aug 16, 2017 at 08:24:19PM +0300, Laurent Pinchart wrote:
> Hi Sakari,
>
> Thank you for the patch.
>
> On Wednesday 16 Aug 2017 20:05:52 Sakari Ailus wrote:
> > struct v4l2_subdev.host_priv is intended to be used by another driver. This
> > is hardly good design but back in the days of platform data was a quick
> > hack to get things done.
> >
> > As the sub-device specific bus information can be stored to the ISP driver
> > specific struct allocated along with v4l2_async_subdev, keep the
> > information there and only there.
> >
> > Signed-off-by: Sakari Ailus
>
> This looks better to me than the previous approach. Please see below for one
> last comment.
>
> > ---
> > since v2.1:
> >
> > - Remove struct isp_async_subdev.sd field as it is now redundant.
> >
> > drivers/media/platform/omap3isp/isp.c | 31 --
> > drivers/media/platform/omap3isp/isp.h | 1 -
> > drivers/media/platform/omap3isp/ispccdc.c | 20 ---
> > drivers/media/platform/omap3isp/ispccp2.c | 4 +++-
> > drivers/media/platform/omap3isp/ispcsi2.c | 3 ++-
> > drivers/media/platform/omap3isp/ispcsiphy.c | 11 +-
> > 6 files changed, 33 insertions(+), 37 deletions(-)
> >
> > diff --git a/drivers/media/platform/omap3isp/isp.c
> > b/drivers/media/platform/omap3isp/isp.c index 6cb1f0495804..27c577fac8e9
> > 100644
> > --- a/drivers/media/platform/omap3isp/isp.c
> > +++ b/drivers/media/platform/omap3isp/isp.c
> > @@ -2188,26 +2188,12 @@ static int isp_fwnodes_parse(struct device *dev,
> > return -EINVAL;
> > }
> >
> > -static int isp_subdev_notifier_bound(struct v4l2_async_notifier *async,
> > -struct v4l2_subdev *subdev,
> > -struct v4l2_async_subdev *asd)
> > -{
> > - struct isp_async_subdev *isd =
> > - container_of(asd, struct isp_async_subdev, asd);
> > -
> > - isd->sd = subdev;
> > - isd->sd->host_priv = &isd->bus;
> > -
> > - return 0;
> > -}
> > -
> > static int isp_subdev_notifier_complete(struct v4l2_async_notifier *async)
> > {
> > struct isp_device *isp = container_of(async, struct isp_device,
> > notifier);
> > struct v4l2_device *v4l2_dev = &isp->v4l2_dev;
> > struct v4l2_subdev *sd;
> > - struct isp_bus_cfg *bus;
> > int ret;
> >
> > ret = media_entity_enum_init(&isp->crashed, &isp->media_dev);
> > @@ -2215,13 +2201,15 @@ static int isp_subdev_notifier_complete(struct
> > v4l2_async_notifier *async) return ret;
> >
> > list_for_each_entry(sd, &v4l2_dev->subdevs, list) {
> > - /* Only try to link entities whose interface was set on bound
> */
> > - if (sd->host_priv) {
> > - bus = (struct isp_bus_cfg *)sd->host_priv;
> > - ret = isp_link_entity(isp, &sd->entity, bus-
> >interface);
> > - if (ret < 0)
> > - return ret;
> > - }
> > + struct isp_async_subdev *isd;
> > +
> > + if (!sd->asd)
> > + continue;
> > +
> > + isd = container_of(sd->asd, struct isp_async_subdev, asd);
> > + ret = isp_link_entity(isp, &sd->entity, isd->bus.interface);
> > + if (ret < 0)
> > + return ret;
> > }
> >
> > ret = v4l2_device_register_subdev_nodes(&isp->v4l2_dev);
> > @@ -2399,7 +2387,6 @@ static int isp_probe(struct platform_device *pdev)
> > if (ret < 0)
> > goto error_register_entities;
> >
> > - isp->notifier.bound = isp_subdev_notifier_bound;
> > isp->notifier.complete = isp_subdev_notifier_complete;
> >
> > ret = v4l2_async_notifier_register(&isp->v4l2_dev, &isp->notifier);
> > diff --git a/drivers/media/platform/omap3isp/isp.h
> > b/drivers/media/platform/omap3isp/isp.h index 2f2ae609c548..25472c81dcdd
> > 100644
> > --- a/drivers/media/platform/omap3isp/isp.h
> > +++ b/drivers/media/platform/omap3isp/isp.h
> > @@ -226,7 +226,6 @@ struct isp_device {
> > };
> >
> > struct isp_async_subdev {
> > - struct v4l2_subdev *sd;
> > struct isp_bus_cfg bus;
> > struct v4l2_async_subdev asd;
> > };
> > diff --git a/drivers/media/platform/omap3isp/ispccdc.c
> > b/drivers/media/platform/omap3isp/ispccdc.c index
> > 4947876cfadf..0145b3dcd7a7 100644
> > --- a/drivers/media/platform/omap3isp/ispccdc.c
> > +++ b/drivers/media/platform/omap3isp/ispccdc.c
> > @@ -1139,8 +1139,12 @@ static void ccdc_configure(struct isp_ccdc_device
> > *ccdc) pad = media_entity_remote_pad(&ccdc->pads[CCDC_PAD_SINK]);
> > sensor = media_entity_to_v4l2_subdev(pad->entity);
> > if (ccdc->input == CCDC_INPUT_PARALLEL) {
> > - parcfg = &((struct isp_bus_cfg *)sensor->host_priv)
> > - ->bus.parallel;
> > + struct isp_pipeline *pipe =
> > + to_isp_pipeline(&ccdc->subdev.entity);
> > +
> > + parcfg = &container_of(pipe->external-
[PATCH v2.3 5/5] omap3isp: Quit using struct v4l2_subdev.host_priv field
struct v4l2_subdev.host_priv is intended to be used by another driver. This
is hardly good design but back in the days of platform data was a quick
hack to get things done.
As the sub-device specific bus information can be stored to the ISP driver
specific struct allocated along with v4l2_async_subdev, keep the
information there and only there.
Signed-off-by: Sakari Ailus
---
since v2.2:
- Introduce a macro v4l2_subdev_to_bus_cfg() in order to remove
non-trivial usage in places where conversion from sub-device to ISP bus
configuration is needed.
drivers/media/platform/omap3isp/isp.c | 29 +++--
drivers/media/platform/omap3isp/isp.h | 4 +++-
drivers/media/platform/omap3isp/ispccdc.c | 12 ++--
drivers/media/platform/omap3isp/ispccp2.c | 3 ++-
drivers/media/platform/omap3isp/ispcsi2.c | 2 +-
drivers/media/platform/omap3isp/ispcsiphy.c | 8 +++-
6 files changed, 22 insertions(+), 36 deletions(-)
diff --git a/drivers/media/platform/omap3isp/isp.c
b/drivers/media/platform/omap3isp/isp.c
index 6cb1f0495804..c3014c82d64d 100644
--- a/drivers/media/platform/omap3isp/isp.c
+++ b/drivers/media/platform/omap3isp/isp.c
@@ -2188,26 +2188,12 @@ static int isp_fwnodes_parse(struct device *dev,
return -EINVAL;
}
-static int isp_subdev_notifier_bound(struct v4l2_async_notifier *async,
-struct v4l2_subdev *subdev,
-struct v4l2_async_subdev *asd)
-{
- struct isp_async_subdev *isd =
- container_of(asd, struct isp_async_subdev, asd);
-
- isd->sd = subdev;
- isd->sd->host_priv = &isd->bus;
-
- return 0;
-}
-
static int isp_subdev_notifier_complete(struct v4l2_async_notifier *async)
{
struct isp_device *isp = container_of(async, struct isp_device,
notifier);
struct v4l2_device *v4l2_dev = &isp->v4l2_dev;
struct v4l2_subdev *sd;
- struct isp_bus_cfg *bus;
int ret;
ret = media_entity_enum_init(&isp->crashed, &isp->media_dev);
@@ -2215,13 +2201,13 @@ static int isp_subdev_notifier_complete(struct
v4l2_async_notifier *async)
return ret;
list_for_each_entry(sd, &v4l2_dev->subdevs, list) {
- /* Only try to link entities whose interface was set on bound */
- if (sd->host_priv) {
- bus = (struct isp_bus_cfg *)sd->host_priv;
- ret = isp_link_entity(isp, &sd->entity, bus->interface);
- if (ret < 0)
- return ret;
- }
+ if (!sd->asd)
+ continue;
+
+ ret = isp_link_entity(isp, &sd->entity,
+ v4l2_subdev_to_bus_cfg(sd)->interface);
+ if (ret < 0)
+ return ret;
}
ret = v4l2_device_register_subdev_nodes(&isp->v4l2_dev);
@@ -2399,7 +2385,6 @@ static int isp_probe(struct platform_device *pdev)
if (ret < 0)
goto error_register_entities;
- isp->notifier.bound = isp_subdev_notifier_bound;
isp->notifier.complete = isp_subdev_notifier_complete;
ret = v4l2_async_notifier_register(&isp->v4l2_dev, &isp->notifier);
diff --git a/drivers/media/platform/omap3isp/isp.h
b/drivers/media/platform/omap3isp/isp.h
index 2f2ae609c548..e528df6efc09 100644
--- a/drivers/media/platform/omap3isp/isp.h
+++ b/drivers/media/platform/omap3isp/isp.h
@@ -226,11 +226,13 @@ struct isp_device {
};
struct isp_async_subdev {
- struct v4l2_subdev *sd;
struct isp_bus_cfg bus;
struct v4l2_async_subdev asd;
};
+#define v4l2_subdev_to_bus_cfg(sd) \
+ (&container_of((sd)->asd, struct isp_async_subdev, asd)->bus)
+
#define v4l2_dev_to_isp_device(dev) \
container_of(dev, struct isp_device, v4l2_dev)
diff --git a/drivers/media/platform/omap3isp/ispccdc.c
b/drivers/media/platform/omap3isp/ispccdc.c
index 4947876cfadf..80fed9526945 100644
--- a/drivers/media/platform/omap3isp/ispccdc.c
+++ b/drivers/media/platform/omap3isp/ispccdc.c
@@ -1139,7 +1139,8 @@ static void ccdc_configure(struct isp_ccdc_device *ccdc)
pad = media_entity_remote_pad(&ccdc->pads[CCDC_PAD_SINK]);
sensor = media_entity_to_v4l2_subdev(pad->entity);
if (ccdc->input == CCDC_INPUT_PARALLEL) {
- parcfg = &((struct isp_bus_cfg *)sensor->host_priv)
+ parcfg = &v4l2_subdev_to_bus_cfg(
+ to_isp_pipeline(&ccdc->subdev.entity)->external)
->bus.parallel;
ccdc->bt656 = parcfg->bt656;
}
@@ -2412,11 +2413,10 @@ static int ccdc_link_validate(struct v4l2_subdev *sd,
/* We've got a parallel sensor here. */
if (ccdc->input == CCDC_INPUT_PARALLEL) {
- struct isp_parallel_cfg *parcfg =
- &
Re: [PATCH v2.3 5/5] omap3isp: Quit using struct v4l2_subdev.host_priv field
Hi Sakari,
Thank you for the patch.
On Wednesday 16 Aug 2017 20:39:43 Sakari Ailus wrote:
> struct v4l2_subdev.host_priv is intended to be used by another driver. This
> is hardly good design but back in the days of platform data was a quick
> hack to get things done.
>
> As the sub-device specific bus information can be stored to the ISP driver
> specific struct allocated along with v4l2_async_subdev, keep the
> information there and only there.
>
> Signed-off-by: Sakari Ailus
> ---
> since v2.2:
>
> - Introduce a macro v4l2_subdev_to_bus_cfg() in order to remove
> non-trivial usage in places where conversion from sub-device to ISP bus
> configuration is needed.
>
> drivers/media/platform/omap3isp/isp.c | 29 ++
> drivers/media/platform/omap3isp/isp.h | 4 +++-
> drivers/media/platform/omap3isp/ispccdc.c | 12 ++--
> drivers/media/platform/omap3isp/ispccp2.c | 3 ++-
> drivers/media/platform/omap3isp/ispcsi2.c | 2 +-
> drivers/media/platform/omap3isp/ispcsiphy.c | 8 +++-
> 6 files changed, 22 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/media/platform/omap3isp/isp.c
> b/drivers/media/platform/omap3isp/isp.c index 6cb1f0495804..c3014c82d64d
> 100644
> --- a/drivers/media/platform/omap3isp/isp.c
> +++ b/drivers/media/platform/omap3isp/isp.c
> @@ -2188,26 +2188,12 @@ static int isp_fwnodes_parse(struct device *dev,
> return -EINVAL;
> }
>
> -static int isp_subdev_notifier_bound(struct v4l2_async_notifier *async,
> - struct v4l2_subdev *subdev,
> - struct v4l2_async_subdev *asd)
> -{
> - struct isp_async_subdev *isd =
> - container_of(asd, struct isp_async_subdev, asd);
> -
> - isd->sd = subdev;
> - isd->sd->host_priv = &isd->bus;
> -
> - return 0;
> -}
> -
> static int isp_subdev_notifier_complete(struct v4l2_async_notifier *async)
> {
> struct isp_device *isp = container_of(async, struct isp_device,
> notifier);
> struct v4l2_device *v4l2_dev = &isp->v4l2_dev;
> struct v4l2_subdev *sd;
> - struct isp_bus_cfg *bus;
> int ret;
>
> ret = media_entity_enum_init(&isp->crashed, &isp->media_dev);
> @@ -2215,13 +2201,13 @@ static int isp_subdev_notifier_complete(struct
> v4l2_async_notifier *async) return ret;
>
> list_for_each_entry(sd, &v4l2_dev->subdevs, list) {
> - /* Only try to link entities whose interface was set on bound
*/
> - if (sd->host_priv) {
> - bus = (struct isp_bus_cfg *)sd->host_priv;
> - ret = isp_link_entity(isp, &sd->entity, bus-
>interface);
> - if (ret < 0)
> - return ret;
> - }
> + if (!sd->asd)
> + continue;
> +
> + ret = isp_link_entity(isp, &sd->entity,
> + v4l2_subdev_to_bus_cfg(sd)->interface);
> + if (ret < 0)
> + return ret;
> }
>
> ret = v4l2_device_register_subdev_nodes(&isp->v4l2_dev);
> @@ -2399,7 +2385,6 @@ static int isp_probe(struct platform_device *pdev)
> if (ret < 0)
> goto error_register_entities;
>
> - isp->notifier.bound = isp_subdev_notifier_bound;
> isp->notifier.complete = isp_subdev_notifier_complete;
>
> ret = v4l2_async_notifier_register(&isp->v4l2_dev, &isp->notifier);
> diff --git a/drivers/media/platform/omap3isp/isp.h
> b/drivers/media/platform/omap3isp/isp.h index 2f2ae609c548..e528df6efc09
> 100644
> --- a/drivers/media/platform/omap3isp/isp.h
> +++ b/drivers/media/platform/omap3isp/isp.h
> @@ -226,11 +226,13 @@ struct isp_device {
> };
>
> struct isp_async_subdev {
> - struct v4l2_subdev *sd;
> struct isp_bus_cfg bus;
> struct v4l2_async_subdev asd;
> };
>
> +#define v4l2_subdev_to_bus_cfg(sd) \
> + (&container_of((sd)->asd, struct isp_async_subdev, asd)->bus)
> +
> #define v4l2_dev_to_isp_device(dev) \
> container_of(dev, struct isp_device, v4l2_dev)
>
> diff --git a/drivers/media/platform/omap3isp/ispccdc.c
> b/drivers/media/platform/omap3isp/ispccdc.c index
> 4947876cfadf..80fed9526945 100644
> --- a/drivers/media/platform/omap3isp/ispccdc.c
> +++ b/drivers/media/platform/omap3isp/ispccdc.c
> @@ -1139,7 +1139,8 @@ static void ccdc_configure(struct isp_ccdc_device
> *ccdc) pad = media_entity_remote_pad(&ccdc->pads[CCDC_PAD_SINK]);
> sensor = media_entity_to_v4l2_subdev(pad->entity);
> if (ccdc->input == CCDC_INPUT_PARALLEL) {
> - parcfg = &((struct isp_bus_cfg *)sensor->host_priv)
> + parcfg = &v4l2_subdev_to_bus_cfg(
> + to_isp_pipeline(&ccdc->subdev.entity)->external)
> ->bus.parallel;
> ccdc->bt656 = parcfg->bt656;
> }
> @@ -2412,11 +2413,10 @@ static int ccdc_link_validate(s
Re: [PATCH 0/3] AS3645A flash support
Hi Sakari,
On 08/16/2017 02:54 PM, Sakari Ailus wrote:
> Hi everyone,
>
> This set adds support for the AS3645A flash driver which can be found e.g.
> in Nokia N9.
>
> The set depeds on the flash patches here so I'd prefer to merge this
> through mediatree.
>
> https://git.linuxtv.org/sailus/media_tree.git/log/?h=flash>
>
> Jacek: would that be ok for you?
No problem.
>
> Since the RFC set:
>
> - Add back the DT binding documentation I lost long ago.
>
> - Use colon (":") in the default names instead of a space.
>
> Sakari Ailus (3):
> dt: bindings: Document DT bindings for Analog devices as3645a
> leds: as3645a: Add LED flash class driver
> arm: dts: omap3: N9/N950: Add AS3645A camera flash
>
> .../devicetree/bindings/leds/ams,as3645a.txt | 56 ++
> MAINTAINERS| 6 +
> arch/arm/boot/dts/omap3-n9.dts | 1 +
> arch/arm/boot/dts/omap3-n950-n9.dtsi | 14 +
> arch/arm/boot/dts/omap3-n950.dts | 1 +
> drivers/leds/Kconfig | 8 +
> drivers/leds/Makefile | 1 +
> drivers/leds/leds-as3645a.c| 785
> +
> 8 files changed, 872 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/leds/ams,as3645a.txt
> create mode 100644 drivers/leds/leds-as3645a.c
>
--
Best regards,
Jacek Anaszewski
Re: [PATCH 1/3] dt: bindings: Document DT bindings for Analog devices as3645a
Hi Sakari,
Thanks for the patch. One issue below.
On 08/16/2017 02:55 PM, Sakari Ailus wrote:
> From: Sakari Ailus
>
> Signed-off-by: Sakari Ailus
> ---
> .../devicetree/bindings/leds/ams,as3645a.txt | 56
> ++
> 1 file changed, 56 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/leds/ams,as3645a.txt
>
> diff --git a/Documentation/devicetree/bindings/leds/ams,as3645a.txt
> b/Documentation/devicetree/bindings/leds/ams,as3645a.txt
> new file mode 100644
> index ..00066e3f9036
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/leds/ams,as3645a.txt
> @@ -0,0 +1,56 @@
> +Analog devices AS3645A device tree bindings
> +
> +The AS3645A flash LED controller can drive two LEDs, one high current
> +flash LED and one indicator LED. The high current flash LED can be
> +used in torch mode as well.
> +
> +Ranges below noted as [a, b] are closed ranges between a and b, i.e. a
> +and b are included in the range.
> +
> +
> +Required properties
> +===
> +
> +compatible : Must be "ams,as3645a".
> +reg : The I2C address of the device. Typically 0x30.
> +
> +
> +Required properties of the "flash" child node
> +=
> +
> +flash-timeout-us: Flash timeout in microseconds. The value must be in
> + the range [10, 85] and divisible by 5.
> +flash-max-microamp: Maximum flash current in microamperes. Has to be
> + in the range between [20, 50] and
> + divisible by 2.
> +led-max-microamp: Maximum torch (assist) current in microamperes. The
> + value must be in the range between [2, 16] and
> + divisible by 2.
> +ams,input-max-microamp: Maximum flash controller input current. The
> + value must be in the range [125, 200]
> + and divisible by 5.
> +
> +
> +Required properties of the "indicator" child node
> +=
> +
> +led-max-microamp: Maximum indicator current. The allowed values are
> + 2500, 5000, 7500 and 1.
Most LED bindings mention also optional label property in the form:
- label : See Documentation/devicetree/bindings/leds/common.txt
> +
> +Example
> +===
> +
> + as3645a: flash@30 {
> + reg = <0x30>;
> + compatible = "ams,as3645a";
> + flash {
label = "as3645a:flash";
> + flash-timeout-us = <15>;
> + flash-max-microamp = <32>;
> + led-max-microamp = <6>;
> + ams,input-max-microamp = <175>;
> + };
> + indicator {
label = "as3645a:indicator";
> + led-max-microamp = <1>;
> + };
> + };
>
--
Best regards,
Jacek Anaszewski
Re: [PATCH 2/3] leds: as3645a: Add LED flash class driver
Hi Sakari, Thanks for the patch. I have few more remarks regarding LED class device naming and pm handling below. On 08/16/2017 02:55 PM, Sakari Ailus wrote: > From: Sakari Ailus > > Add a LED flash class driver for the as3654a flash controller. A V4L2 flash > driver for it already exists (drivers/media/i2c/as3645a.c), and this driver > is based on that. > > Signed-off-by: Sakari Ailus > --- > MAINTAINERS | 6 + > drivers/leds/Kconfig| 8 + > drivers/leds/Makefile | 1 + > drivers/leds/leds-as3645a.c | 785 > > 4 files changed, 800 insertions(+) > create mode 100644 drivers/leds/leds-as3645a.c > > diff --git a/MAINTAINERS b/MAINTAINERS > index 931abca006b7..8f40ba2e5303 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -2124,6 +2124,12 @@ F: arch/arm64/ > F: Documentation/arm64/ > > AS3645A LED FLASH CONTROLLER DRIVER > +M: Sakari Ailus > +L: linux-l...@vger.kernel.org > +S: Maintained > +F: drivers/leds/leds-as3645a.c > + > +AS3645A LED FLASH CONTROLLER DRIVER > M: Laurent Pinchart > L: linux-media@vger.kernel.org > T: git git://linuxtv.org/media_tree.git > diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig > index 594b24d410c3..bad3a4098104 100644 > --- a/drivers/leds/Kconfig > +++ b/drivers/leds/Kconfig > @@ -58,6 +58,14 @@ config LEDS_AAT1290 > help >This option enables support for the LEDs on the AAT1290. > > +config LEDS_AS3645A > + tristate "AS3645A LED flash controller support" > + depends on I2C && LEDS_CLASS_FLASH > + help > + Enable LED flash class support for AS3645A LED flash > + controller. V4L2 flash API is provided as well if > + CONFIG_V4L2_FLASH_API is enabled. > + > config LEDS_BCM6328 > tristate "LED Support for Broadcom BCM6328" > depends on LEDS_CLASS > diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile > index 909dae62ba05..7d7b26552923 100644 > --- a/drivers/leds/Makefile > +++ b/drivers/leds/Makefile > @@ -8,6 +8,7 @@ obj-$(CONFIG_LEDS_TRIGGERS) += led-triggers.o > # LED Platform Drivers > obj-$(CONFIG_LEDS_88PM860X) += leds-88pm860x.o > obj-$(CONFIG_LEDS_AAT1290) += leds-aat1290.o > +obj-$(CONFIG_LEDS_AS3645A) += leds-as3645a.o > obj-$(CONFIG_LEDS_BCM6328) += leds-bcm6328.o > obj-$(CONFIG_LEDS_BCM6358) += leds-bcm6358.o > obj-$(CONFIG_LEDS_BD2802)+= leds-bd2802.o > diff --git a/drivers/leds/leds-as3645a.c b/drivers/leds/leds-as3645a.c > new file mode 100644 > index ..2335510a08e1 > --- /dev/null > +++ b/drivers/leds/leds-as3645a.c > @@ -0,0 +1,785 @@ > +/* > + * drivers/leds/leds-as3645a.c - AS3645A and LM3555 flash controllers driver > + * > + * Copyright (C) 2008-2011 Nokia Corporation > + * Copyright (c) 2011, 2017 Intel Corporation. > + * > + * Based on drivers/media/i2c/as3645a.c. > + * > + * Contact: Sakari Ailus > + * > + * 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. > + * > + * This program is distributed in the hope that it will be useful, but > + * WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + * General Public License for more details. > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include > + > +#define AS_TIMER_US_TO_CODE(t) (((t) / 1000 - 100) / > 50) > +#define AS_TIMER_CODE_TO_US(c) ((50 * (c) + 100) * > 1000) > + > +/* Register definitions */ > + > +/* Read-only Design info register: Reset state: 0001 */ > +#define AS_DESIGN_INFO_REG 0x00 > +#define AS_DESIGN_INFO_FACTORY(x)(((x) >> 4)) > +#define AS_DESIGN_INFO_MODEL(x) ((x) & 0x0f) > + > +/* Read-only Version control register: Reset state: > + * for first engineering samples > + */ > +#define AS_VERSION_CONTROL_REG 0x01 > +#define AS_VERSION_CONTROL_RFU(x)(((x) >> 4)) > +#define AS_VERSION_CONTROL_VERSION(x)((x) & 0x0f) > + > +/* Read / Write (Indicator and timer register): Reset state: > */ > +#define AS_INDICATOR_AND_TIMER_REG 0x02 > +#define AS_INDICATOR_AND_TIMER_TIMEOUT_SHIFT 0 > +#define AS_INDICATOR_AND_TIMER_VREF_SHIFT4 > +#define AS_INDICATOR_AND_TIMER_INDICATOR_SHIFT 6 > + > +/* Read / Write (Current set register): Reset state: 0110 1001 */ > +#define AS_CURRENT_SET_REG 0x03 > +#define AS_CURRENT_ASSIST_LIGHT_SHIFT0 > +#define AS_CURRENT_LED_DET_ON(1 << 3) > +#define AS_CURRENT_FLASH_CURRENT_SHIFT 4 > + > +/* Read / Write
[PATCH v2.4 5/5] omap3isp: Quit using struct v4l2_subdev.host_priv field
struct v4l2_subdev.host_priv is intended to be used by another driver. This
is hardly good design but back in the days of platform data was a quick
hack to get things done.
As the sub-device specific bus information can be stored to the ISP driver
specific struct allocated along with v4l2_async_subdev, keep the
information there and only there.
Signed-off-by: Sakari Ailus
---
since v2.3:
- Fix obtaining the external entity in ispccdc.c.
drivers/media/platform/omap3isp/isp.c | 29 +++--
drivers/media/platform/omap3isp/isp.h | 4 +++-
drivers/media/platform/omap3isp/ispccdc.c | 12 ++--
drivers/media/platform/omap3isp/ispccp2.c | 3 ++-
drivers/media/platform/omap3isp/ispcsi2.c | 2 +-
drivers/media/platform/omap3isp/ispcsiphy.c | 8 +++-
6 files changed, 22 insertions(+), 36 deletions(-)
diff --git a/drivers/media/platform/omap3isp/isp.c
b/drivers/media/platform/omap3isp/isp.c
index 6cb1f0495804..c3014c82d64d 100644
--- a/drivers/media/platform/omap3isp/isp.c
+++ b/drivers/media/platform/omap3isp/isp.c
@@ -2188,26 +2188,12 @@ static int isp_fwnodes_parse(struct device *dev,
return -EINVAL;
}
-static int isp_subdev_notifier_bound(struct v4l2_async_notifier *async,
-struct v4l2_subdev *subdev,
-struct v4l2_async_subdev *asd)
-{
- struct isp_async_subdev *isd =
- container_of(asd, struct isp_async_subdev, asd);
-
- isd->sd = subdev;
- isd->sd->host_priv = &isd->bus;
-
- return 0;
-}
-
static int isp_subdev_notifier_complete(struct v4l2_async_notifier *async)
{
struct isp_device *isp = container_of(async, struct isp_device,
notifier);
struct v4l2_device *v4l2_dev = &isp->v4l2_dev;
struct v4l2_subdev *sd;
- struct isp_bus_cfg *bus;
int ret;
ret = media_entity_enum_init(&isp->crashed, &isp->media_dev);
@@ -2215,13 +2201,13 @@ static int isp_subdev_notifier_complete(struct
v4l2_async_notifier *async)
return ret;
list_for_each_entry(sd, &v4l2_dev->subdevs, list) {
- /* Only try to link entities whose interface was set on bound */
- if (sd->host_priv) {
- bus = (struct isp_bus_cfg *)sd->host_priv;
- ret = isp_link_entity(isp, &sd->entity, bus->interface);
- if (ret < 0)
- return ret;
- }
+ if (!sd->asd)
+ continue;
+
+ ret = isp_link_entity(isp, &sd->entity,
+ v4l2_subdev_to_bus_cfg(sd)->interface);
+ if (ret < 0)
+ return ret;
}
ret = v4l2_device_register_subdev_nodes(&isp->v4l2_dev);
@@ -2399,7 +2385,6 @@ static int isp_probe(struct platform_device *pdev)
if (ret < 0)
goto error_register_entities;
- isp->notifier.bound = isp_subdev_notifier_bound;
isp->notifier.complete = isp_subdev_notifier_complete;
ret = v4l2_async_notifier_register(&isp->v4l2_dev, &isp->notifier);
diff --git a/drivers/media/platform/omap3isp/isp.h
b/drivers/media/platform/omap3isp/isp.h
index 2f2ae609c548..e528df6efc09 100644
--- a/drivers/media/platform/omap3isp/isp.h
+++ b/drivers/media/platform/omap3isp/isp.h
@@ -226,11 +226,13 @@ struct isp_device {
};
struct isp_async_subdev {
- struct v4l2_subdev *sd;
struct isp_bus_cfg bus;
struct v4l2_async_subdev asd;
};
+#define v4l2_subdev_to_bus_cfg(sd) \
+ (&container_of((sd)->asd, struct isp_async_subdev, asd)->bus)
+
#define v4l2_dev_to_isp_device(dev) \
container_of(dev, struct isp_device, v4l2_dev)
diff --git a/drivers/media/platform/omap3isp/ispccdc.c
b/drivers/media/platform/omap3isp/ispccdc.c
index 4947876cfadf..1c12a8420801 100644
--- a/drivers/media/platform/omap3isp/ispccdc.c
+++ b/drivers/media/platform/omap3isp/ispccdc.c
@@ -1139,7 +1139,8 @@ static void ccdc_configure(struct isp_ccdc_device *ccdc)
pad = media_entity_remote_pad(&ccdc->pads[CCDC_PAD_SINK]);
sensor = media_entity_to_v4l2_subdev(pad->entity);
if (ccdc->input == CCDC_INPUT_PARALLEL) {
- parcfg = &((struct isp_bus_cfg *)sensor->host_priv)
+ parcfg = &v4l2_subdev_to_bus_cfg(
+ to_isp_pipeline(&ccdc->subdev.entity)->external)
->bus.parallel;
ccdc->bt656 = parcfg->bt656;
}
@@ -2412,11 +2413,10 @@ static int ccdc_link_validate(struct v4l2_subdev *sd,
/* We've got a parallel sensor here. */
if (ccdc->input == CCDC_INPUT_PARALLEL) {
- struct isp_parallel_cfg *parcfg =
- &((struct isp_bus_cfg *)
- media_entity_to_v4l2_subdev(link->source->entity)
-
Re: [PATCH 1/3] dt: bindings: Document DT bindings for Analog devices as3645a
Hi Jacek,
Thanks for the review.
On Wed, Aug 16, 2017 at 10:27:27PM +0200, Jacek Anaszewski wrote:
> Hi Sakari,
>
> Thanks for the patch. One issue below.
>
> On 08/16/2017 02:55 PM, Sakari Ailus wrote:
> > From: Sakari Ailus
> >
> > Signed-off-by: Sakari Ailus
> > ---
> > .../devicetree/bindings/leds/ams,as3645a.txt | 56
> > ++
> > 1 file changed, 56 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/leds/ams,as3645a.txt
> >
> > diff --git a/Documentation/devicetree/bindings/leds/ams,as3645a.txt
> > b/Documentation/devicetree/bindings/leds/ams,as3645a.txt
> > new file mode 100644
> > index ..00066e3f9036
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/leds/ams,as3645a.txt
> > @@ -0,0 +1,56 @@
> > +Analog devices AS3645A device tree bindings
> > +
> > +The AS3645A flash LED controller can drive two LEDs, one high current
> > +flash LED and one indicator LED. The high current flash LED can be
> > +used in torch mode as well.
> > +
> > +Ranges below noted as [a, b] are closed ranges between a and b, i.e. a
> > +and b are included in the range.
> > +
> > +
> > +Required properties
> > +===
> > +
> > +compatible : Must be "ams,as3645a".
> > +reg: The I2C address of the device. Typically 0x30.
> > +
> > +
> > +Required properties of the "flash" child node
> > +=
> > +
> > +flash-timeout-us: Flash timeout in microseconds. The value must be in
> > + the range [10, 85] and divisible by 5.
> > +flash-max-microamp: Maximum flash current in microamperes. Has to be
> > + in the range between [20, 50] and
> > + divisible by 2.
> > +led-max-microamp: Maximum torch (assist) current in microamperes. The
> > + value must be in the range between [2, 16] and
> > + divisible by 2.
> > +ams,input-max-microamp: Maximum flash controller input current. The
> > + value must be in the range [125, 200]
> > + and divisible by 5.
> > +
> > +
> > +Required properties of the "indicator" child node
> > +=
> > +
> > +led-max-microamp: Maximum indicator current. The allowed values are
> > + 2500, 5000, 7500 and 1.
>
> Most LED bindings mention also optional label property in the form:
>
> - label : See Documentation/devicetree/bindings/leds/common.txt
>
> > +
> > +Example
> > +===
> > +
> > + as3645a: flash@30 {
> > + reg = <0x30>;
> > + compatible = "ams,as3645a";
> > + flash {
>
> label = "as3645a:flash";
>
> > + flash-timeout-us = <15>;
> > + flash-max-microamp = <32>;
> > + led-max-microamp = <6>;
> > + ams,input-max-microamp = <175>;
> > + };
> > + indicator {
>
> label = "as3645a:indicator";
>
> > + led-max-microamp = <1>;
> > + };
> > + };
> >
I'll make the above fixes for v2 (and add flash child node label property).
--
Regards,
Sakari Ailus
e-mail: sakari.ai...@iki.fi
Re: [PATCH v3 1/4] i2c: add helpers to ease DMA handling
Hi Jonathan, > I like the basic idea of this patch set a lot btw! Thanks! > Jonathan Could you delete irrelevant parts of the message, please? I nearly missed your other comment which would have been a great loss! > I'm trying to get my head around whether this is a sufficient set of > conditions > for a dma safe buffer and I'm not sure it is. > > We go to a lot of effort with SPI buffers to ensure they are in their own > cache > lines. Do we not need to do the same here? The issue would be the > classic case of embedding a buffer inside a larger structure which is on > the stack. Need the magic of __cacheline_aligned to force it into it's > own line for devices with dma-incoherent caches. > DMA-API-HOWTO.txt (not read that for a while at it is a rather good > at describing this stuff these days!) > > So that one is easy enough to check by checking if it is cache line aligned, > but what do you do to know there is nothing much after it? Thank you, really! This patch series addressed all cases I have created, but I also wondered if there are cases left which I missed. Now, also because you mentioned cacheline alignments, the faint idea of "maybe it could be fragile" became a strong "it is too fragile"! lib/dma-debug.c is >40KB for a reason. I just rewrote this series... > Not sure there is a way around this short of auditing i2c drivers to > change any that do this. ... to do something like this, more precisely: an opt-in approach. I introduce a new flag for i2c_msg, namely I2C_M_DMA_SAFE which can opt-in DMA if we know the i2c_msg buffer is DMA safe. I finished a proof-of-concept this evening and pushed it here: git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git renesas/topic/i2c-core-dma-rfc-v4 I will test it on HW tomorrow and send it out, then. There are still some bits missing, but for getting opinions, it should be good enough. Thanks and kind regards, Wolfram signature.asc Description: PGP signature
Re: [PATCH v2 1/8] v4l: vsp1: Protect fragments against overflow
Hi Kieran,
Thank you for the patch.
On Monday 14 Aug 2017 16:13:24 Kieran Bingham wrote:
> The fragment write function relies on the code never asking it to
> write more than the entries available in the list.
>
> Currently with each list body containing 256 entries, this is fine,
> but we can reduce this number greatly saving memory.
>
> In preparation of this - add a level of protection to catch any
> buffer overflows.
>
> Signed-off-by: Kieran Bingham
> ---
> drivers/media/platform/vsp1/vsp1_dl.c | 8
> 1 file changed, 8 insertions(+)
>
> diff --git a/drivers/media/platform/vsp1/vsp1_dl.c
> b/drivers/media/platform/vsp1/vsp1_dl.c index 8b5cbb6b7a70..cb4625ae13c2
> 100644
> --- a/drivers/media/platform/vsp1/vsp1_dl.c
> +++ b/drivers/media/platform/vsp1/vsp1_dl.c
> @@ -50,6 +50,7 @@ struct vsp1_dl_entry {
> * @dma: DMA address of the entries
> * @size: size of the DMA memory in bytes
> * @num_entries: number of stored entries
> + * @max_entries: number of entries available
> */
> struct vsp1_dl_body {
> struct list_head list;
> @@ -60,6 +61,7 @@ struct vsp1_dl_body {
> size_t size;
>
> unsigned int num_entries;
> + unsigned int max_entries;
> };
>
> /**
> @@ -138,6 +140,7 @@ static int vsp1_dl_body_init(struct vsp1_device *vsp1,
>
> dlb->vsp1 = vsp1;
> dlb->size = size;
> + dlb->max_entries = num_entries;
>
> dlb->entries = dma_alloc_wc(vsp1->bus_master, dlb->size, &dlb->dma,
> GFP_KERNEL);
> @@ -220,6 +223,11 @@ void vsp1_dl_fragment_free(struct vsp1_dl_body *dlb)
> */
> void vsp1_dl_fragment_write(struct vsp1_dl_body *dlb, u32 reg, u32 data)
> {
> + if (unlikely(dlb->num_entries >= dlb->max_entries)) {
> + WARN_ONCE(true, "DLB size exceeded (max %u)", dlb-
>max_entries);
> + return;
> + }
How about
if (WARN_ONCE(dlb->num_entries >= dlb->max_entries,
"DLB size exceeded (max %u)", dlb->max_entries))
return;
(WARN_ONCE contains the unlikely() already)
I'm not fussed either way,
Reviewed-by: Laurent Pinchart
> dlb->entries[dlb->num_entries].addr = reg;
> dlb->entries[dlb->num_entries].data = data;
> dlb->num_entries++;
--
Regards,
Laurent Pinchart
[PATCH] [media] coda/imx-vdoa: Check for platform_get_resource() error
From: Fabio Estevam
platform_get_resource() may fail and in this case a NULL dereference
will occur.
Prevent this from happening by returning an error on
platform_get_resource() failure.
Fixes: b0444f18e0b18abce ("[media] coda: add i.MX6 VDOA driver")
Signed-off-by: Fabio Estevam
---
drivers/media/platform/coda/imx-vdoa.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/media/platform/coda/imx-vdoa.c
b/drivers/media/platform/coda/imx-vdoa.c
index df9b716..8eb3e0c 100644
--- a/drivers/media/platform/coda/imx-vdoa.c
+++ b/drivers/media/platform/coda/imx-vdoa.c
@@ -314,6 +314,8 @@ static int vdoa_probe(struct platform_device *pdev)
return PTR_ERR(vdoa->regs);
res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
+ if (!res)
+ return -EINVAL;
vdoa->irq = devm_request_threaded_irq(&pdev->dev, res->start, NULL,
vdoa_irq_handler, IRQF_ONESHOT,
"vdoa", vdoa);
--
2.7.4
Re: [PATCH 1/2] media: ov7670: Add entity pads initialization
Hi Sakari,
On 2017/8/16 0:23, Sakari Ailus wrote:
Hi Wenyou,
On Thu, Aug 10, 2017 at 05:06:44PM +0800, Wenyou Yang wrote:
Add the media entity pads initialization.
Signed-off-by: Wenyou Yang
The patch itself seems fine. However the driver is lacking support for
get_fmt which I think would be necessary for the user space API to work
properly. Without sub-device nodes it might not have been an issue with
certain master drivers.
Could you add that in v2, in a separate patch before this one, please?
Okay, I will do.
Thank you for your review.
---
drivers/media/i2c/ov7670.c | 13 -
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c
index e88549f0e704..5c8460ee65c3 100644
--- a/drivers/media/i2c/ov7670.c
+++ b/drivers/media/i2c/ov7670.c
@@ -213,6 +213,7 @@ struct ov7670_devtype {
struct ov7670_format_struct; /* coming later */
struct ov7670_info {
struct v4l2_subdev sd;
+ struct media_pad pad;
struct v4l2_ctrl_handler hdl;
struct {
/* gain cluster */
@@ -1688,14 +1689,23 @@ static int ov7670_probe(struct i2c_client *client,
v4l2_ctrl_auto_cluster(2, &info->auto_exposure,
V4L2_EXPOSURE_MANUAL, false);
v4l2_ctrl_cluster(2, &info->saturation);
+
+ info->pad.flags = MEDIA_PAD_FL_SOURCE;
+ info->sd.entity.function = MEDIA_ENT_F_CAM_SENSOR;
+ ret = media_entity_pads_init(&info->sd.entity, 1, &info->pad);
+ if (ret < 0)
+ goto hdl_free;
+
v4l2_ctrl_handler_setup(&info->hdl);
ret = v4l2_async_register_subdev(&info->sd);
if (ret < 0)
- goto hdl_free;
+ goto entity_cleanup;
return 0;
+entity_cleanup:
+ media_entity_cleanup(&info->sd.entity);
hdl_free:
v4l2_ctrl_handler_free(&info->hdl);
clk_disable:
@@ -1712,6 +1722,7 @@ static int ov7670_remove(struct i2c_client *client)
v4l2_device_unregister_subdev(sd);
v4l2_ctrl_handler_free(&info->hdl);
clk_disable_unprepare(info->clk);
+ media_entity_cleanup(&info->sd.entity);
return 0;
}
--
2.13.0
Best Regards,
Wenyou Yang
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: Thu Aug 17 05:00:15 CEST 2017 media-tree git hash:ec0c3ec497cabbf3bfa03a9eb5edcc252190a4e0 media_build git hash: a03e89634b47f570039c7d6931cd751777d4bba1 v4l-utils git hash: 5a67da05fded64b5f678033c16196799e134c62c gcc version:i686-linux-gcc (GCC) 7.1.0 sparse version: v0.5.0 smatch version: v0.5.0-3553-g78b2ea6 host hardware: x86_64 host os:4.11.0-164 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-blackfin-bf561: OK linux-git-i686: WARNINGS linux-git-m32r: OK linux-git-mips: OK linux-git-powerpc64: OK linux-git-sh: OK linux-git-x86_64: WARNINGS linux-2.6.36.4-i686: WARNINGS linux-2.6.37.6-i686: WARNINGS linux-2.6.38.8-i686: WARNINGS linux-2.6.39.4-i686: WARNINGS linux-3.0.60-i686: WARNINGS linux-3.1.10-i686: WARNINGS linux-3.2.37-i686: WARNINGS linux-3.3.8-i686: WARNINGS linux-3.4.27-i686: ERRORS linux-3.5.7-i686: WARNINGS linux-3.6.11-i686: WARNINGS linux-3.7.4-i686: WARNINGS linux-3.8-i686: WARNINGS linux-3.9.2-i686: WARNINGS linux-3.10.1-i686: WARNINGS linux-3.11.1-i686: WARNINGS linux-3.12.67-i686: WARNINGS linux-3.13.11-i686: WARNINGS linux-3.14.9-i686: WARNINGS linux-3.15.2-i686: WARNINGS linux-3.16.7-i686: WARNINGS linux-3.17.8-i686: ERRORS linux-3.18.7-i686: ERRORS linux-3.19-i686: WARNINGS linux-4.0.9-i686: WARNINGS linux-4.1.33-i686: WARNINGS linux-4.2.8-i686: WARNINGS linux-4.3.6-i686: WARNINGS linux-4.4.22-i686: WARNINGS linux-4.5.7-i686: WARNINGS linux-4.6.7-i686: WARNINGS linux-4.7.5-i686: WARNINGS linux-4.8-i686: WARNINGS linux-4.9.26-i686: WARNINGS linux-4.10.14-i686: WARNINGS linux-4.11-i686: WARNINGS linux-4.12.1-i686: WARNINGS linux-2.6.36.4-x86_64: WARNINGS linux-2.6.37.6-x86_64: WARNINGS linux-2.6.38.8-x86_64: WARNINGS linux-2.6.39.4-x86_64: WARNINGS linux-3.0.60-x86_64: WARNINGS linux-3.1.10-x86_64: WARNINGS linux-3.2.37-x86_64: WARNINGS linux-3.3.8-x86_64: WARNINGS linux-3.4.27-x86_64: ERRORS linux-3.5.7-x86_64: WARNINGS linux-3.6.11-x86_64: WARNINGS linux-3.7.4-x86_64: WARNINGS linux-3.8-x86_64: WARNINGS linux-3.9.2-x86_64: WARNINGS linux-3.10.1-x86_64: WARNINGS linux-3.11.1-x86_64: WARNINGS linux-3.12.67-x86_64: WARNINGS linux-3.13.11-x86_64: WARNINGS linux-3.14.9-x86_64: WARNINGS linux-3.15.2-x86_64: WARNINGS linux-3.16.7-x86_64: WARNINGS linux-3.17.8-x86_64: WARNINGS linux-3.18.7-x86_64: WARNINGS linux-3.19-x86_64: WARNINGS linux-4.0.9-x86_64: WARNINGS linux-4.1.33-x86_64: WARNINGS linux-4.2.8-x86_64: WARNINGS linux-4.3.6-x86_64: WARNINGS linux-4.4.22-x86_64: WARNINGS linux-4.5.7-x86_64: WARNINGS linux-4.6.7-x86_64: WARNINGS linux-4.7.5-x86_64: WARNINGS linux-4.8-x86_64: WARNINGS linux-4.9.26-x86_64: WARNINGS linux-4.10.14-x86_64: WARNINGS linux-4.11-x86_64: WARNINGS linux-4.12.1-x86_64: WARNINGS apps: WARNINGS spec-git: OK Detailed results are available here: http://www.xs4all.nl/~hverkuil/logs/Thursday.log Full logs are available here: http://www.xs4all.nl/~hverkuil/logs/Thursday.tar.bz2 The Media Infrastructure API from this daily build is here: http://www.xs4all.nl/~hverkuil/spec/index.html
Re: [PATCH 4/5] stih-cec: use CEC_CAP_DEFAULTS
Benjamin, Can you please review this patch and the stm32-cec patch? Thanks! Hans On 08/04/2017 12:41 PM, Hans Verkuil wrote: > From: Hans Verkuil > > Use the new CEC_CAP_DEFAULTS define in this driver. > This also adds the CEC_CAP_RC capability which was missing here > (and this is also the reason for this new define, to avoid missing > such capabilities). > > Signed-off-by: Hans Verkuil > --- > drivers/media/platform/sti/cec/stih-cec.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > diff --git a/drivers/media/platform/sti/cec/stih-cec.c > b/drivers/media/platform/sti/cec/stih-cec.c > index ce7964c71b50..dc221527fd05 100644 > --- a/drivers/media/platform/sti/cec/stih-cec.c > +++ b/drivers/media/platform/sti/cec/stih-cec.c > @@ -351,9 +351,7 @@ static int stih_cec_probe(struct platform_device *pdev) > } > > cec->adap = cec_allocate_adapter(&sti_cec_adap_ops, cec, > - CEC_NAME, > - CEC_CAP_LOG_ADDRS | CEC_CAP_PASSTHROUGH | > - CEC_CAP_TRANSMIT, CEC_MAX_LOG_ADDRS); > + CEC_NAME, CEC_CAP_DEFAULTS, CEC_MAX_LOG_ADDRS); > if (IS_ERR(cec->adap)) > return PTR_ERR(cec->adap); > >
[GIT PULL FOR v4.14] cec: rename uAPI defines, fixes
Hi Mauro, The second patch renames two CEC events in the public API. Since these were introduced for 4.14, now is the time to do the rename before they become part of the ABI. While working with and working on the cec-gpio driver to debug CEC issues I realized that optionally being able to monitor the HDMI HPD (hotplug detect) pin as well is very useful. So besides monitoring the CEC pin it will also be possible to monitor the HPD pin in the future. That means that the pin event has to tell with pin has an event, CEC or HPD. Hence the rename while I still can. The last patch is a fix for the irq handling in cec-pin.c which was broken. This only affects the cec-gpio driver which isn't merged yet (expected for 4.15), but it is good to get this fixed now. Regards, Hans The following changes since commit ec0c3ec497cabbf3bfa03a9eb5edcc252190a4e0: media: ddbridge: split code into multiple files (2017-08-09 12:17:01 -0400) are available in the git repository at: git://linuxtv.org/hverkuil/media_tree.git cec-rename for you to fetch changes up to 36fa912800ef8129b6c8c9caffb74a84ff5be36d: cec-pin: fix irq handling (2017-08-17 08:43:50 +0200) Hans Verkuil (3): s5p-cec: use CEC_CAP_DEFAULTS cec: rename pin events/function cec-pin: fix irq handling Documentation/media/uapi/cec/cec-ioc-adap-g-caps.rst | 2 +- Documentation/media/uapi/cec/cec-ioc-dqevent.rst | 8 Documentation/media/uapi/cec/cec-ioc-g-mode.rst | 2 +- drivers/media/cec/cec-adap.c | 7 --- drivers/media/cec/cec-api.c | 4 ++-- drivers/media/cec/cec-pin.c | 39 --- drivers/media/platform/s5p-cec/s5p_cec.c | 7 ++- include/media/cec-pin.h | 6 +- include/media/cec.h | 9 + include/uapi/linux/cec.h | 4 ++-- 10 files changed, 50 insertions(+), 38 deletions(-)
