Re: [PATCH 2/2] drm: adv7511: Add support for i2c_new_secondary_device

2018-01-22 Thread Kieran Bingham 
On 22/01/18 13:00, Lars-Peter Clausen wrote:
> On 01/22/2018 01:50 PM, Kieran Bingham wrote:
>> The ADV7511 has four 256-byte maps that can be accessed via the main I²C
>> ports. Each map has it own I²C address and acts as a standard slave
>> device on the I²C bus.
>>
>> Allow a device tree node to override the default addresses so that
>> address conflicts with other devices on the same bus may be resolved at
>> the board description level.
>>
>> Signed-off-by: Kieran Bingham 
> 
> I've been working on the same thing, but you've beat me to it! Patch looks
> mostly OK, but I think you are missing this piece:
> 
> https://github.com/analogdevicesinc/linux/commit/ba9b57507cb78724a606eb24104e22fea942437d#diff-2cf1828c644e351adefabe9509410400L553

Ah yes - Thanks, - you're correct - I had missed that bit.

Added locally for a v2.
--
Kieran

>> ---
>>  .../bindings/display/bridge/adi,adv7511.txt| 10 +-
>>  drivers/gpu/drm/bridge/adv7511/adv7511.h   |  4 +++
>>  drivers/gpu/drm/bridge/adv7511/adv7511_drv.c   | 36 
>> ++
>>  3 files changed, 37 insertions(+), 13 deletions(-)
>>
>> diff --git 
>> a/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt 
>> b/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
>> index 0047b1394c70..f6bb9f6d3f48 100644
>> --- a/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
>> +++ b/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
>> @@ -70,6 +70,9 @@ Optional properties:
>>rather than generate its own timings for HDMI output.
>>  - clocks: from common clock binding: reference to the CEC clock.
>>  - clock-names: from common clock binding: must be "cec".
>> +- reg-names : Names of maps with programmable addresses.
>> +It can contain any map needing a non-default address.
>> +Possible maps names are : "main", "edid", "cec", "packet"
>>  
>>  Required nodes:
>>  
>> @@ -88,7 +91,12 @@ Example
>>  
>>  adv7511w: hdmi@39 {
>>  compatible = "adi,adv7511w";
>> -reg = <39>;
>> +/*
>> + * The EDID page will be accessible on address 0x66 on the i2c
>> + * bus. All other maps continue to use their default addresses.
>> + */
>> +reg = <0x39 0x66>;
>> +reg-names = "main", "edid";
>>  interrupt-parent = <>;
>>  interrupts = <29 IRQ_TYPE_EDGE_FALLING>;
>>  clocks = <_clock>;
>> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511.h 
>> b/drivers/gpu/drm/bridge/adv7511/adv7511.h
>> index d034b2cb5eee..7d81ce3808e0 100644
>> --- a/drivers/gpu/drm/bridge/adv7511/adv7511.h
>> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511.h
>> @@ -53,8 +53,10 @@
>>  #define ADV7511_REG_POWER   0x41
>>  #define ADV7511_REG_STATUS  0x42
>>  #define ADV7511_REG_EDID_I2C_ADDR   0x43
>> +#define ADV7511_REG_EDID_I2C_ADDR_DEFAULT   0x3f
>>  #define ADV7511_REG_PACKET_ENABLE1  0x44
>>  #define ADV7511_REG_PACKET_I2C_ADDR 0x45
>> +#define ADV7511_REG_PACKET_I2C_ADDR_DEFAULT 0x38
>>  #define ADV7511_REG_DSD_ENABLE  0x46
>>  #define ADV7511_REG_VIDEO_INPUT_CFG20x48
>>  #define ADV7511_REG_INFOFRAME_UPDATE0x4a
>> @@ -89,6 +91,7 @@
>>  #define ADV7511_REG_TMDS_CLOCK_INV  0xde
>>  #define ADV7511_REG_ARC_CTRL0xdf
>>  #define ADV7511_REG_CEC_I2C_ADDR0xe1
>> +#define ADV7511_REG_CEC_I2C_ADDR_DEFAULT0x3c
>>  #define ADV7511_REG_CEC_CTRL0xe2
>>  #define ADV7511_REG_CHIP_ID_HIGH0xf5
>>  #define ADV7511_REG_CHIP_ID_LOW 0xf6
>> @@ -322,6 +325,7 @@ struct adv7511 {
>>  struct i2c_client *i2c_main;
>>  struct i2c_client *i2c_edid;
>>  struct i2c_client *i2c_cec;
>> +struct i2c_client *i2c_packet;
>>  
>>  struct regmap *regmap;
>>  struct regmap *regmap_cec;
>> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c 
>> b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
>> index efa29db5fc2b..7ec33837752b 100644
>> --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
>> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
>> @@ -969,8 +969,8 @@ static int adv7511_init_cec_regmap(struct adv7511 *adv)
>>  {
>>  int ret;
>>  
>> -adv->i2c_cec = i2c_new_dummy(adv->i2c_main->adapter,
>> - adv->i2c_main->addr - 1);
>> +adv->i2c_cec = i2c_new_secondary_device(adv->i2c_main, "cec",
>> +ADV7511_REG_CEC_I2C_ADDR_DEFAULT);
>>  if (!adv->i2c_cec)
>>  return -ENOMEM;
>>  i2c_set_clientdata(adv->i2c_cec, adv);
>> @@ -1082,8 +1082,6 @@ static int adv7511_probe(struct i2c_client *i2c, const 
>> struct i2c_device_id *id)
>>  struct adv7511_link_config link_config;
>>  struct adv7511 *adv7511;
>>  struct device *dev = >dev;
>> -unsigned int

Re: [PATCH v2] serdev: add method to set parity

2018-01-22 Thread Greg Kroah-Hartman 
On Mon, Jan 22, 2018 at 07:23:00PM +0100, Marcel Holtmann wrote:
> Hi Rob,
> 
> > Adds serdev_device_set_parity() and an implementation for ttyport.
> > The interface uses an enum with the values SERIAL_PARITY_NONE,
> > SERIAL_PARITY_EVEN and SERIAL_PARITY_ODD.
> > 
> > Signed-off-by: Ulrich Hecht 
> > Reviewed-by: Sebastian Reichel 
> > ---
> > 
> > Hi!
> > 
> > This revision addresses Johan's comments (see below for details) and adds
> > Sebastian's Reviewed-by tag. Thank you for your reviews!
> > 
> > CU
> > Uli
> > 
> > 
> > Changes since v1:
> > - added Reviewed-by tag
> > - expanded commit message
> > - shuffled stuff around to keep line-setting bits together
> > - clear CMSPAR
> > - (hopefully) detect errors correctly by checking tty->termios after call
> >  to tty_set_termios().
> > 
> > 
> > drivers/tty/serdev/core.c   | 12 
> > drivers/tty/serdev/serdev-ttyport.c | 24 
> > include/linux/serdev.h  | 10 ++
> > 3 files changed, 46 insertions(+)
> 
> if there are no objections, I would like to just take this through 
> bluetooth-next tree.

Acked-by: Greg Kroah-Hartman

Re: [PATCH v2] serdev: add method to set parity

2018-01-22 Thread Johan Hovold 
On Mon, Jan 22, 2018 at 06:56:32PM +0100, Ulrich Hecht wrote:
> Adds serdev_device_set_parity() and an implementation for ttyport.
> The interface uses an enum with the values SERIAL_PARITY_NONE,
> SERIAL_PARITY_EVEN and SERIAL_PARITY_ODD.
> 
> Signed-off-by: Ulrich Hecht 
> Reviewed-by: Sebastian Reichel 
> ---
> 
> Hi!
> 
> This revision addresses Johan's comments (see below for details) and adds
> Sebastian's Reviewed-by tag. Thank you for your reviews!
> 
> CU
> Uli
> 
> 
> Changes since v1:
> - added Reviewed-by tag
> - expanded commit message
> - shuffled stuff around to keep line-setting bits together
> - clear CMSPAR
> - (hopefully) detect errors correctly by checking tty->termios after call
>   to tty_set_termios().

Thanks for the v2. Looks good to me now.

Reviewed-by: Johan Hovold 

Johan

Re: [PATCH v2] serdev: add method to set parity

2018-01-22 Thread Marcel Holtmann 
Hi Rob,

> Adds serdev_device_set_parity() and an implementation for ttyport.
> The interface uses an enum with the values SERIAL_PARITY_NONE,
> SERIAL_PARITY_EVEN and SERIAL_PARITY_ODD.
> 
> Signed-off-by: Ulrich Hecht 
> Reviewed-by: Sebastian Reichel 
> ---
> 
> Hi!
> 
> This revision addresses Johan's comments (see below for details) and adds
> Sebastian's Reviewed-by tag. Thank you for your reviews!
> 
> CU
> Uli
> 
> 
> Changes since v1:
> - added Reviewed-by tag
> - expanded commit message
> - shuffled stuff around to keep line-setting bits together
> - clear CMSPAR
> - (hopefully) detect errors correctly by checking tty->termios after call
>  to tty_set_termios().
> 
> 
> drivers/tty/serdev/core.c   | 12 
> drivers/tty/serdev/serdev-ttyport.c | 24 
> include/linux/serdev.h  | 10 ++
> 3 files changed, 46 insertions(+)

if there are no objections, I would like to just take this through 
bluetooth-next tree.

Regards

Marcel

[PATCH v2] serdev: add method to set parity

2018-01-22 Thread Ulrich Hecht 
Adds serdev_device_set_parity() and an implementation for ttyport.
The interface uses an enum with the values SERIAL_PARITY_NONE,
SERIAL_PARITY_EVEN and SERIAL_PARITY_ODD.

Signed-off-by: Ulrich Hecht 
Reviewed-by: Sebastian Reichel 
---

Hi!

This revision addresses Johan's comments (see below for details) and adds
Sebastian's Reviewed-by tag. Thank you for your reviews!

CU
Uli


Changes since v1:
- added Reviewed-by tag
- expanded commit message
- shuffled stuff around to keep line-setting bits together
- clear CMSPAR
- (hopefully) detect errors correctly by checking tty->termios after call
  to tty_set_termios().


 drivers/tty/serdev/core.c   | 12 
 drivers/tty/serdev/serdev-ttyport.c | 24 
 include/linux/serdev.h  | 10 ++
 3 files changed, 46 insertions(+)

diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c
index 5dc88f6..ceee0ca 100644
--- a/drivers/tty/serdev/core.c
+++ b/drivers/tty/serdev/core.c
@@ -257,6 +257,18 @@ void serdev_device_set_flow_control(struct serdev_device 
*serdev, bool enable)
 }
 EXPORT_SYMBOL_GPL(serdev_device_set_flow_control);
 
+int serdev_device_set_parity(struct serdev_device *serdev,
+enum serdev_parity parity)
+{
+   struct serdev_controller *ctrl = serdev->ctrl;
+
+   if (!ctrl || !ctrl->ops->set_parity)
+   return -ENOTSUPP;
+
+   return ctrl->ops->set_parity(ctrl, parity);
+}
+EXPORT_SYMBOL_GPL(serdev_device_set_parity);
+
 void serdev_device_wait_until_sent(struct serdev_device *serdev, long timeout)
 {
struct serdev_controller *ctrl = serdev->ctrl;
diff --git a/drivers/tty/serdev/serdev-ttyport.c 
b/drivers/tty/serdev/serdev-ttyport.c
index a5abb05..fa16729 100644
--- a/drivers/tty/serdev/serdev-ttyport.c
+++ b/drivers/tty/serdev/serdev-ttyport.c
@@ -194,6 +194,29 @@ static void ttyport_set_flow_control(struct 
serdev_controller *ctrl, bool enable
tty_set_termios(tty, );
 }
 
+static int ttyport_set_parity(struct serdev_controller *ctrl,
+ enum serdev_parity parity)
+{
+   struct serport *serport = serdev_controller_get_drvdata(ctrl);
+   struct tty_struct *tty = serport->tty;
+   struct ktermios ktermios = tty->termios;
+
+   ktermios.c_cflag &= ~(PARENB | PARODD | CMSPAR);
+   if (parity != SERDEV_PARITY_NONE) {
+   ktermios.c_cflag |= PARENB;
+   if (parity == SERDEV_PARITY_ODD)
+   ktermios.c_cflag |= PARODD;
+   }
+
+   tty_set_termios(tty, );
+
+   if ((tty->termios.c_cflag & (PARENB | PARODD | CMSPAR)) !=
+   (ktermios.c_cflag & (PARENB | PARODD | CMSPAR)))
+   return -EINVAL;
+
+   return 0;
+}
+
 static void ttyport_wait_until_sent(struct serdev_controller *ctrl, long 
timeout)
 {
struct serport *serport = serdev_controller_get_drvdata(ctrl);
@@ -231,6 +254,7 @@ static const struct serdev_controller_ops ctrl_ops = {
.open = ttyport_open,
.close = ttyport_close,
.set_flow_control = ttyport_set_flow_control,
+   .set_parity = ttyport_set_parity,
.set_baudrate = ttyport_set_baudrate,
.wait_until_sent = ttyport_wait_until_sent,
.get_tiocm = ttyport_get_tiocm,
diff --git a/include/linux/serdev.h b/include/linux/serdev.h
index 48d8ce2..f153b2c 100644
--- a/include/linux/serdev.h
+++ b/include/linux/serdev.h
@@ -78,6 +78,12 @@ static inline struct serdev_device_driver 
*to_serdev_device_driver(struct device
return container_of(d, struct serdev_device_driver, driver);
 }
 
+enum serdev_parity {
+   SERDEV_PARITY_NONE,
+   SERDEV_PARITY_EVEN,
+   SERDEV_PARITY_ODD,
+};
+
 /*
  * serdev controller structures
  */
@@ -88,6 +94,7 @@ struct serdev_controller_ops {
int (*open)(struct serdev_controller *);
void (*close)(struct serdev_controller *);
void (*set_flow_control)(struct serdev_controller *, bool);
+   int (*set_parity)(struct serdev_controller *, enum serdev_parity);
unsigned int (*set_baudrate)(struct serdev_controller *, unsigned int);
void (*wait_until_sent)(struct serdev_controller *, long);
int (*get_tiocm)(struct serdev_controller *);
@@ -301,6 +308,9 @@ static inline int serdev_device_set_rts(struct 
serdev_device *serdev, bool enabl
return serdev_device_set_tiocm(serdev, 0, TIOCM_RTS);
 }
 
+int serdev_device_set_parity(struct serdev_device *serdev,
+enum serdev_parity parity);
+
 /*
  * serdev hooks into TTY core
  */
-- 
2.7.4

Re: [PATCH 0/2] Use demuxer for i2c1 and i2c5

2018-01-22 Thread Wolfram Sang 

> > > On top of renesas-dev branch, I am using the below i2c-rcar driver related
> > patches.
> > >
> > > https://www.mail-archive.com/linux-renesas-soc@vger.kernel.org/msg2227
> > > 8.html
> >
> > Those should be enough. They are not?
> 
> Yes. I have tested without demuxer patches, I confirm, it can recover the bus.

That is great news! Thanks for the additional testing. I was a little
worried that the 'bus stalled' detection might have failed for you, but
it seems it works for your case as well.

With that put aside, it is still valid, to add the demuxer to the DTS
file in case you want to switch to bitbanged GPIO at runtime for some
reason. Only the commit message would need updating then, however,
because it is not relevant for bus recovery.



signature.asc
Description: PGP signature

RE: [PATCH 0/2] Use demuxer for i2c1 and i2c5

2018-01-22 Thread Biju Das 
> Subject: Re: [PATCH 0/2] Use demuxer for i2c1 and i2c5

[>]
> > How do i2c-bus gets details of gpio lines associated with i2c bus for bus
> recovery?
>
> As you can see in the code, you don't need GPIOs if you can control SDA and 
> SCL
> directly. The I2C IP can do this.

I agree. Thanks for correcting me.

> > On top of renesas-dev branch, I am using the below i2c-rcar driver related
> patches.
> >
> > https://www.mail-archive.com/linux-renesas-soc@vger.kernel.org/msg2227
> > 8.html
>
> Those should be enough. They are not?

Yes. I have tested without demuxer patches, I confirm, it can recover the bus.




Renesas Electronics Europe Ltd, Dukes Meadow, Millboard Road, Bourne End, 
Buckinghamshire, SL8 5FH, UK. Registered in England & Wales under Registered 
No. 04586709.

Re: [PATCH 2/2] drm: adv7511: Add support for i2c_new_secondary_device

2018-01-22 Thread Lars-Peter Clausen 
On 01/22/2018 01:50 PM, Kieran Bingham wrote:
> The ADV7511 has four 256-byte maps that can be accessed via the main I²C
> ports. Each map has it own I²C address and acts as a standard slave
> device on the I²C bus.
> 
> Allow a device tree node to override the default addresses so that
> address conflicts with other devices on the same bus may be resolved at
> the board description level.
> 
> Signed-off-by: Kieran Bingham 

I've been working on the same thing, but you've beat me to it! Patch looks
mostly OK, but I think you are missing this piece:

https://github.com/analogdevicesinc/linux/commit/ba9b57507cb78724a606eb24104e22fea942437d#diff-2cf1828c644e351adefabe9509410400L553

> ---
>  .../bindings/display/bridge/adi,adv7511.txt| 10 +-
>  drivers/gpu/drm/bridge/adv7511/adv7511.h   |  4 +++
>  drivers/gpu/drm/bridge/adv7511/adv7511_drv.c   | 36 
> ++
>  3 files changed, 37 insertions(+), 13 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt 
> b/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
> index 0047b1394c70..f6bb9f6d3f48 100644
> --- a/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
> +++ b/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
> @@ -70,6 +70,9 @@ Optional properties:
>rather than generate its own timings for HDMI output.
>  - clocks: from common clock binding: reference to the CEC clock.
>  - clock-names: from common clock binding: must be "cec".
> +- reg-names : Names of maps with programmable addresses.
> + It can contain any map needing a non-default address.
> + Possible maps names are : "main", "edid", "cec", "packet"
>  
>  Required nodes:
>  
> @@ -88,7 +91,12 @@ Example
>  
>   adv7511w: hdmi@39 {
>   compatible = "adi,adv7511w";
> - reg = <39>;
> + /*
> +  * The EDID page will be accessible on address 0x66 on the i2c
> +  * bus. All other maps continue to use their default addresses.
> +  */
> + reg = <0x39 0x66>;
> + reg-names = "main", "edid";
>   interrupt-parent = <>;
>   interrupts = <29 IRQ_TYPE_EDGE_FALLING>;
>   clocks = <_clock>;
> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511.h 
> b/drivers/gpu/drm/bridge/adv7511/adv7511.h
> index d034b2cb5eee..7d81ce3808e0 100644
> --- a/drivers/gpu/drm/bridge/adv7511/adv7511.h
> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511.h
> @@ -53,8 +53,10 @@
>  #define ADV7511_REG_POWER0x41
>  #define ADV7511_REG_STATUS   0x42
>  #define ADV7511_REG_EDID_I2C_ADDR0x43
> +#define ADV7511_REG_EDID_I2C_ADDR_DEFAULT0x3f
>  #define ADV7511_REG_PACKET_ENABLE1   0x44
>  #define ADV7511_REG_PACKET_I2C_ADDR  0x45
> +#define ADV7511_REG_PACKET_I2C_ADDR_DEFAULT  0x38
>  #define ADV7511_REG_DSD_ENABLE   0x46
>  #define ADV7511_REG_VIDEO_INPUT_CFG2 0x48
>  #define ADV7511_REG_INFOFRAME_UPDATE 0x4a
> @@ -89,6 +91,7 @@
>  #define ADV7511_REG_TMDS_CLOCK_INV   0xde
>  #define ADV7511_REG_ARC_CTRL 0xdf
>  #define ADV7511_REG_CEC_I2C_ADDR 0xe1
> +#define ADV7511_REG_CEC_I2C_ADDR_DEFAULT 0x3c
>  #define ADV7511_REG_CEC_CTRL 0xe2
>  #define ADV7511_REG_CHIP_ID_HIGH 0xf5
>  #define ADV7511_REG_CHIP_ID_LOW  0xf6
> @@ -322,6 +325,7 @@ struct adv7511 {
>   struct i2c_client *i2c_main;
>   struct i2c_client *i2c_edid;
>   struct i2c_client *i2c_cec;
> + struct i2c_client *i2c_packet;
>  
>   struct regmap *regmap;
>   struct regmap *regmap_cec;
> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c 
> b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> index efa29db5fc2b..7ec33837752b 100644
> --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> @@ -969,8 +969,8 @@ static int adv7511_init_cec_regmap(struct adv7511 *adv)
>  {
>   int ret;
>  
> - adv->i2c_cec = i2c_new_dummy(adv->i2c_main->adapter,
> -  adv->i2c_main->addr - 1);
> + adv->i2c_cec = i2c_new_secondary_device(adv->i2c_main, "cec",
> + ADV7511_REG_CEC_I2C_ADDR_DEFAULT);
>   if (!adv->i2c_cec)
>   return -ENOMEM;
>   i2c_set_clientdata(adv->i2c_cec, adv);
> @@ -1082,8 +1082,6 @@ static int adv7511_probe(struct i2c_client *i2c, const 
> struct i2c_device_id *id)
>   struct adv7511_link_config link_config;
>   struct adv7511 *adv7511;
>   struct device *dev = >dev;
> - unsigned int main_i2c_addr = i2c->addr << 1;
> - unsigned int edid_i2c_addr = main_i2c_addr + 4;
>   unsigned int val;
>   int ret;
>  
> @@ -1153,24 +1151,35 @@ static int adv7511_probe(struct i2c_client *i2c, 
> const struct

[PATCH 1/2] media: adv7604: Add support for i2c_new_secondary_device

2018-01-22 Thread Kieran Bingham 
From: Jean-Michel Hautbois 

The ADV7604 has thirteen 256-byte maps that can be accessed via the main
I²C ports. Each map has it own I²C address and acts as a standard slave
device on the I²C bus.

Allow a device tree node to override the default addresses so that
address conflicts with other devices on the same bus may be resolved at
the board description level.

Signed-off-by: Jean-Michel Hautbois 
[Kieran: Re-adapted for mainline]
Signed-off-by: Kieran Bingham 
---
Based upon the original posting :
  https://lkml.org/lkml/2014/10/22/469
---
 .../devicetree/bindings/media/i2c/adv7604.txt  | 18 ++-
 drivers/media/i2c/adv7604.c| 60 ++
 2 files changed, 55 insertions(+), 23 deletions(-)

diff --git a/Documentation/devicetree/bindings/media/i2c/adv7604.txt 
b/Documentation/devicetree/bindings/media/i2c/adv7604.txt
index 9cbd92eb5d05..b64e313dcc66 100644
--- a/Documentation/devicetree/bindings/media/i2c/adv7604.txt
+++ b/Documentation/devicetree/bindings/media/i2c/adv7604.txt
@@ -13,7 +13,11 @@ Required Properties:
 - "adi,adv7611" for the ADV7611
 - "adi,adv7612" for the ADV7612
 
-  - reg: I2C slave address
+  - reg: I2C slave addresses
+The ADV76xx has up to thirteen 256-byte maps that can be accessed via the
+main I²C ports. Each map has it own I²C address and acts as a standard
+slave device on the I²C bus. The main address is mandatory, others are
+optional and revert to defaults if not specified.
 
   - hpd-gpios: References to the GPIOs that control the HDMI hot-plug
 detection pins, one per HDMI input. The active flag indicates the GPIO
@@ -35,6 +39,11 @@ Optional Properties:
 
   - reset-gpios: Reference to the GPIO connected to the device's reset pin.
   - default-input: Select which input is selected after reset.
+  - reg-names : Names of maps with programmable addresses.
+   It can contain any map needing a non-default address.
+   Possible maps names are :
+ "main", "avlink", "cec", "infoframe", "esdp", "dpp", "afe",
+ "rep", "edid", "hdmi", "test", "cp", "vdp"
 
 Optional Endpoint Properties:
 
@@ -52,7 +61,12 @@ Example:
 
hdmi_receiver@4c {
compatible = "adi,adv7611";
-   reg = <0x4c>;
+   /*
+* The edid page will be accessible @ 0x66 on the i2c bus. All
+* other maps will retain their default addresses.
+*/
+   reg = <0x4c 0x66>;
+   reg-names "main", "edid";
 
reset-gpios = < 0 GPIO_ACTIVE_LOW>;
hpd-gpios = < 2 GPIO_ACTIVE_HIGH>;
diff --git a/drivers/media/i2c/adv7604.c b/drivers/media/i2c/adv7604.c
index 1544920ec52d..c346b9a8fb57 100644
--- a/drivers/media/i2c/adv7604.c
+++ b/drivers/media/i2c/adv7604.c
@@ -2734,6 +2734,27 @@ static const struct v4l2_ctrl_config 
adv76xx_ctrl_free_run_color = {
 
 /* --- */
 
+struct adv76xx_register {
+   const char *name;
+   u8 default_addr;
+};
+
+static const struct adv76xx_register adv76xx_secondary_names[] = {
+   [ADV76XX_PAGE_IO] = { "main", 0x4c },
+   [ADV7604_PAGE_AVLINK] = { "avlink", 0x42 },
+   [ADV76XX_PAGE_CEC] = { "cec", 0x40 },
+   [ADV76XX_PAGE_INFOFRAME] = { "infoframe", 0x3e },
+   [ADV7604_PAGE_ESDP] = { "esdp", 0x38 },
+   [ADV7604_PAGE_DPP] = { "dpp", 0x3c },
+   [ADV76XX_PAGE_AFE] = { "afe", 0x26 },
+   [ADV76XX_PAGE_REP] = { "rep", 0x32 },
+   [ADV76XX_PAGE_EDID] = { "edid", 0x36 },
+   [ADV76XX_PAGE_HDMI] = { "hdmi", 0x34 },
+   [ADV76XX_PAGE_TEST] = { "test", 0x30 },
+   [ADV76XX_PAGE_CP] = { "cp", 0x22 },
+   [ADV7604_PAGE_VDP] = { "vdp", 0x24 },
+};
+
 static int adv76xx_core_init(struct v4l2_subdev *sd)
 {
struct adv76xx_state *state = to_state(sd);
@@ -2834,13 +2855,26 @@ static void adv76xx_unregister_clients(struct 
adv76xx_state *state)
 }
 
 static struct i2c_client *adv76xx_dummy_client(struct v4l2_subdev *sd,
-   u8 addr, u8 io_reg)
+  unsigned int i)
 {
struct i2c_client *client = v4l2_get_subdevdata(sd);
+   struct adv76xx_state *state = to_state(sd);
+   struct adv76xx_platform_data *pdata = >pdata;
+   unsigned int io_reg = 0xf2 + i;
+   struct i2c_client *new_client;
+
+   if (pdata && pdata->i2c_addresses[i])
+   new_client = i2c_new_dummy(client->adapter,
+  pdata->i2c_addresses[i]);
+   else
+   new_client = i2c_new_secondary_device(client,
+   adv76xx_secondary_names[i].name,
+   adv76xx_secondary_names[i].default_addr);
 
-   if (addr)
-

[PATCH 0/2] Add support for i2c_new_secondary_device

2018-01-22 Thread Kieran Bingham 
Back in 2014, Jean-Michel provided patches [0] to implement a means of
describing software defined I2C addresses for devices through the DT nodes.

The patch to implement the function "i2c_new_secondary_device()" was integrated,
but the corresponding driver update didn't get applied.

This short series re-bases Jean-Michel's patch to mainline for the ADV7604 
driver
in linux-media, and also provides a patch for the ADV7511 DRM Bridge driver 
taking
the same approach.

This series allows us to define the I2C address allocations of these devices in
the device tree for the Renesas D3 platform where these two devices reside on
the same bus and conflict with each other presently..

[0] https://lkml.org/lkml/2014/10/22/468
[1] https://lkml.org/lkml/2014/10/22/469

Jean-Michel Hautbois (1):
  media: adv7604: Add support for i2c_new_secondary_device

Kieran Bingham (1):
  drm: adv7511: Add support for i2c_new_secondary_device

 .../bindings/display/bridge/adi,adv7511.txt| 10 +++-
 .../devicetree/bindings/media/i2c/adv7604.txt  | 18 ++-
 drivers/gpu/drm/bridge/adv7511/adv7511.h   |  4 ++
 drivers/gpu/drm/bridge/adv7511/adv7511_drv.c   | 36 -
 drivers/media/i2c/adv7604.c| 60 ++
 5 files changed, 92 insertions(+), 36 deletions(-)

-- 
2.7.4

[PATCH 2/2] drm: adv7511: Add support for i2c_new_secondary_device

2018-01-22 Thread Kieran Bingham 
The ADV7511 has four 256-byte maps that can be accessed via the main I²C
ports. Each map has it own I²C address and acts as a standard slave
device on the I²C bus.

Allow a device tree node to override the default addresses so that
address conflicts with other devices on the same bus may be resolved at
the board description level.

Signed-off-by: Kieran Bingham 
---
 .../bindings/display/bridge/adi,adv7511.txt| 10 +-
 drivers/gpu/drm/bridge/adv7511/adv7511.h   |  4 +++
 drivers/gpu/drm/bridge/adv7511/adv7511_drv.c   | 36 ++
 3 files changed, 37 insertions(+), 13 deletions(-)

diff --git a/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt 
b/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
index 0047b1394c70..f6bb9f6d3f48 100644
--- a/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
+++ b/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
@@ -70,6 +70,9 @@ Optional properties:
   rather than generate its own timings for HDMI output.
 - clocks: from common clock binding: reference to the CEC clock.
 - clock-names: from common clock binding: must be "cec".
+- reg-names : Names of maps with programmable addresses.
+   It can contain any map needing a non-default address.
+   Possible maps names are : "main", "edid", "cec", "packet"
 
 Required nodes:
 
@@ -88,7 +91,12 @@ Example
 
adv7511w: hdmi@39 {
compatible = "adi,adv7511w";
-   reg = <39>;
+   /*
+* The EDID page will be accessible on address 0x66 on the i2c
+* bus. All other maps continue to use their default addresses.
+*/
+   reg = <0x39 0x66>;
+   reg-names = "main", "edid";
interrupt-parent = <>;
interrupts = <29 IRQ_TYPE_EDGE_FALLING>;
clocks = <_clock>;
diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511.h 
b/drivers/gpu/drm/bridge/adv7511/adv7511.h
index d034b2cb5eee..7d81ce3808e0 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7511.h
+++ b/drivers/gpu/drm/bridge/adv7511/adv7511.h
@@ -53,8 +53,10 @@
 #define ADV7511_REG_POWER  0x41
 #define ADV7511_REG_STATUS 0x42
 #define ADV7511_REG_EDID_I2C_ADDR  0x43
+#define ADV7511_REG_EDID_I2C_ADDR_DEFAULT  0x3f
 #define ADV7511_REG_PACKET_ENABLE1 0x44
 #define ADV7511_REG_PACKET_I2C_ADDR0x45
+#define ADV7511_REG_PACKET_I2C_ADDR_DEFAULT0x38
 #define ADV7511_REG_DSD_ENABLE 0x46
 #define ADV7511_REG_VIDEO_INPUT_CFG2   0x48
 #define ADV7511_REG_INFOFRAME_UPDATE   0x4a
@@ -89,6 +91,7 @@
 #define ADV7511_REG_TMDS_CLOCK_INV 0xde
 #define ADV7511_REG_ARC_CTRL   0xdf
 #define ADV7511_REG_CEC_I2C_ADDR   0xe1
+#define ADV7511_REG_CEC_I2C_ADDR_DEFAULT   0x3c
 #define ADV7511_REG_CEC_CTRL   0xe2
 #define ADV7511_REG_CHIP_ID_HIGH   0xf5
 #define ADV7511_REG_CHIP_ID_LOW0xf6
@@ -322,6 +325,7 @@ struct adv7511 {
struct i2c_client *i2c_main;
struct i2c_client *i2c_edid;
struct i2c_client *i2c_cec;
+   struct i2c_client *i2c_packet;
 
struct regmap *regmap;
struct regmap *regmap_cec;
diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c 
b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
index efa29db5fc2b..7ec33837752b 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
+++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
@@ -969,8 +969,8 @@ static int adv7511_init_cec_regmap(struct adv7511 *adv)
 {
int ret;
 
-   adv->i2c_cec = i2c_new_dummy(adv->i2c_main->adapter,
-adv->i2c_main->addr - 1);
+   adv->i2c_cec = i2c_new_secondary_device(adv->i2c_main, "cec",
+   ADV7511_REG_CEC_I2C_ADDR_DEFAULT);
if (!adv->i2c_cec)
return -ENOMEM;
i2c_set_clientdata(adv->i2c_cec, adv);
@@ -1082,8 +1082,6 @@ static int adv7511_probe(struct i2c_client *i2c, const 
struct i2c_device_id *id)
struct adv7511_link_config link_config;
struct adv7511 *adv7511;
struct device *dev = >dev;
-   unsigned int main_i2c_addr = i2c->addr << 1;
-   unsigned int edid_i2c_addr = main_i2c_addr + 4;
unsigned int val;
int ret;
 
@@ -1153,24 +1151,35 @@ static int adv7511_probe(struct i2c_client *i2c, const 
struct i2c_device_id *id)
if (ret)
goto uninit_regulators;
 
-   regmap_write(adv7511->regmap, ADV7511_REG_EDID_I2C_ADDR, edid_i2c_addr);
-   regmap_write(adv7511->regmap, ADV7511_REG_PACKET_I2C_ADDR,
-main_i2c_addr - 0xa);
-   regmap_write(adv7511->regmap, ADV7511_REG_CEC_I2C_ADDR,
-main_i2c_addr - 2);
-
adv7511_packet_disable(adv7511, 0x);
 
-

Re: [PATCH 0/2] Use demuxer for i2c1 and i2c5

2018-01-22 Thread Wolfram Sang 

> How do i2c-bus gets details of gpio lines associated with i2c bus for bus 
> recovery?

As you can see in the code, you don't need GPIOs if you can control SDA
and SCL directly. The I2C IP can do this.

> On top of renesas-dev branch, I am using the below i2c-rcar driver related 
> patches.
> 
> https://www.mail-archive.com/linux-renesas-soc@vger.kernel.org/msg22278.html

Those should be enough. They are not?



signature.asc
Description: PGP signature

Re: [PATCH] serdev: add method to set parity

2018-01-22 Thread Marcel Holtmann 
Hi Ulrich,

>> Adds serdev_device_set_parity() and an implementation for ttyport.
> 
> Perhaps you can mention that the interface uses an enum and the three
> settings that you add here.
> 
>> Signed-off-by: Ulrich Hecht 
>> ---
>> Broken out of the "[PATCH 0/6] serdev multiplexing support" series
>> because this kind of functionality is apparently also required
>> for "[RFC v2 0/9] Realtek Bluetooth serdev support (H5 protocol)",
>> which contains an earlier revision of this patch.
> 
> Thanks for submitting this separately.
> 
>> drivers/tty/serdev/core.c   | 12 
>> drivers/tty/serdev/serdev-ttyport.c | 18 ++
>> include/linux/serdev.h  | 10 ++
>> 3 files changed, 40 insertions(+)
>> 
>> diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c
>> index 5dc88f6..1f25896 100644
>> --- a/drivers/tty/serdev/core.c
>> +++ b/drivers/tty/serdev/core.c
>> @@ -290,6 +290,18 @@ int serdev_device_set_tiocm(struct serdev_device 
>> *serdev, int set, int clear)
>> }
>> EXPORT_SYMBOL_GPL(serdev_device_set_tiocm);
>> 
>> +int serdev_device_set_parity(struct serdev_device *serdev,
>> + enum serdev_parity parity)
>> +{
>> +struct serdev_controller *ctrl = serdev->ctrl;
>> +
>> +if (!ctrl || !ctrl->ops->set_parity)
>> +return -ENOTSUPP;
>> +
>> +return ctrl->ops->set_parity(ctrl, parity);
>> +}
>> +EXPORT_SYMBOL_GPL(serdev_device_set_parity);
> 
> Please place the parity functions (and fields) after the set_flow
> equivalents so that we keep the line-setting functionality grouped
> together.
> 
>> +static int ttyport_set_parity(struct serdev_controller *ctrl,
>> +  enum serdev_parity parity)
>> +{
>> +struct serport *serport = serdev_controller_get_drvdata(ctrl);
>> +struct tty_struct *tty = serport->tty;
>> +struct ktermios ktermios = tty->termios;
>> +
>> +ktermios.c_cflag &= ~(PARENB | PARODD);
> 
> You should also clear CMSPAR.
> 
>> +if (parity != SERDEV_PARITY_NONE) {
>> +ktermios.c_cflag |= PARENB;
>> +if (parity == SERDEV_PARITY_ODD)
>> +ktermios.c_cflag |= PARODD;
>> +}
>> +
>> +return tty_set_termios(tty, );
> 
> Note that tty_set_termios() always return 0. You need to verify that you
> got what you requested by looking at the termios after the function
> returns (and possibly return -EINVAL). Not all drivers support (all)
> parity modes.
> 
> Note that we currently do not implement proper error handling for flow
> control, but that should be fixed.
> 
> Looks good otherwise.

can I get an updated patch addressing the comments and also including the 
Reviewed-by tags.

Regards

Marcel

RE: [PATCH 0/2] Use demuxer for i2c1 and i2c5

2018-01-22 Thread Biju Das 
> Subject: Re: [PATCH 0/2] Use demuxer for i2c1 and i2c5
>
> On Mon, Jan 22, 2018 at 11:29:21AM +, Biju Das wrote:
> > We have observed that on iWave RZ/G1E boards, randomly HDMI slave
> > device is holding data line after a soft reboot.
> >
> > We applied demuxer patch for i2c1 and using an oscilloscope verified
> > that i2c recovery works on the failure condition.
>
> Do you really need the demuxer for that?

How do i2c-bus gets details of gpio lines associated with i2c bus for bus 
recovery?

For eg:- i2c1 scl line --> gpio0 11
I2c1 sda line --> gpio0 12

> I posted and just merged bus recovery to the i2c-rcar driver. Did you test 
> that?
> In theory, this should be enough.

On top of renesas-dev branch, I am using the below i2c-rcar driver related 
patches.

https://www.mail-archive.com/linux-renesas-soc@vger.kernel.org/msg22278.html






Renesas Electronics Europe Ltd, Dukes Meadow, Millboard Road, Bourne End, 
Buckinghamshire, SL8 5FH, UK. Registered in England & Wales under Registered 
No. 04586709.

Re: [RFC PATCH v2 0/1] of: easier debugging for node life cycle issues

2018-01-22 Thread Wolfram Sang 
Hi Frank,

> Please go back and read the thread for version 1.  Simply resubmitting a
> forward port is ignoring that whole conversation.
> 
> There is a lot of good info in that thread.  I certainly learned stuff in it.

Yes, I did that and learned stuff, too. My summary of the discussion was:

- you mentioned some drawbacks you saw (like the mixture of trace output
  and printk output)
- most of them look like addressed to me? (e.g. Steven showed a way to redirect
  printk to trace)
- you posted your version (which was, however, marked as "not user friendly"
  even by yourself)
- The discussion stalled over having two approaches

So, I thought reposting would be a good way of finding out if your
concerns were addressed in the discussion or not. If I overlooked
something, I am sorry for that. Still, my intention is to continue the
discussion, not to ignore it. Because as it stands, we don't have such a
debugging mechanism in place currently, and with people working with DT
overlays, I'd think it would be nice to have.

Kind regards,

   Wolfram



signature.asc
Description: PGP signature

Re: [PATCH 0/2] Use demuxer for i2c1 and i2c5

2018-01-22 Thread Wolfram Sang 
On Mon, Jan 22, 2018 at 11:29:21AM +, Biju Das wrote:
> We have observed that on iWave RZ/G1E boards, randomly HDMI slave device
> is holding data line after a soft reboot.
> 
> We applied demuxer patch for i2c1 and using an oscilloscope verified that
> i2c recovery works on the failure condition.

Do you really need the demuxer for that?

I posted and just merged bus recovery to the i2c-rcar driver. Did you
test that? In theory, this should be enough.



signature.asc
Description: PGP signature

[PATCH 0/2] Use demuxer for i2c1 and i2c5

2018-01-22 Thread Biju Das 
We have observed that on iWave RZ/G1E boards, randomly HDMI slave device
is holding data line after a soft reboot.

We applied demuxer patch for i2c1 and using an oscilloscope verified that
i2c recovery works on the failure condition.

This patch depends on the below patch set

https://patchwork.ozlabs.org/cover/857505/

it is tested against renesas-devel-20180122-v4.15-rc9

Biju Das (2):
  ARM: dts: iwg22d-sodimm-dbhd-ca: use demuxer for I2C1
  ARM: dts: iwg22d-sodimm: use demuxer for I2C5

 .../arm/boot/dts/r8a7745-iwg22d-sodimm-dbhd-ca.dts | 96 +-
 arch/arm/boot/dts/r8a7745-iwg22d-sodimm.dts| 44 +++---
 2 files changed, 92 insertions(+), 48 deletions(-)

-- 
2.7.4

[PATCH vY 1/2] ARM: dts: iwg22d-sodimm-dbhd-ca: use demuxer for I2C1

2018-01-22 Thread Biju Das 
Create a separate bus for HDMI related I2C1 and provide fallback to GPIO.

Based on work for r8a7794/Alt by Wolfram Sang: "ARM: dts: alt: use demuxer
 for I2C1"

Add Fixes: 97b94d256d432ba9 ("ARM: dts: iwg22d-sodimm-dbhd-ca: Add HDMI
video output")
Signed-off-by: Biju Das 
Reviewed-by: Chris Paterson 
---
We have observed that randomly HDMI slave device is holding data line after a 
soft reboot.

We applied this patch and using an oscilloscope verified that i2c recovery 
works on the
failure condition.

 .../arm/boot/dts/r8a7745-iwg22d-sodimm-dbhd-ca.dts | 96 +-
 1 file changed, 59 insertions(+), 37 deletions(-)

diff --git a/arch/arm/boot/dts/r8a7745-iwg22d-sodimm-dbhd-ca.dts 
b/arch/arm/boot/dts/r8a7745-iwg22d-sodimm-dbhd-ca.dts
index d34de82..de120a3 100644
--- a/arch/arm/boot/dts/r8a7745-iwg22d-sodimm-dbhd-ca.dts
+++ b/arch/arm/boot/dts/r8a7745-iwg22d-sodimm-dbhd-ca.dts
@@ -19,6 +19,8 @@
serial0 = 
serial4 = 
serial6 = 
+   i2c8 = 
+   i2c10 = 
};
 
cec_clock: cec-clock {
@@ -27,6 +29,16 @@
clock-frequency = <1200>;
};
 
+   gpioi2c1: i2c-8 {
+   #address-cells = <1>;
+   #size-cells = <0>;
+   compatible = "i2c-gpio";
+   status = "disabled";
+   scl-gpios = < 11 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
+   sda-gpios = < 12 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
+   i2c-gpio,delay-us = <5>;
+   };
+
hdmi-out {
compatible = "hdmi-connector";
type = "a";
@@ -37,6 +49,52 @@
};
};
};
+
+   /*
+* A fallback to GPIO is provided for I2C1.
+*/
+   i2chdmi: i2c-10 {
+   compatible = "i2c-demux-pinctrl";
+   i2c-parent = <>, <>;
+   i2c-bus-name = "i2c-hdmi";
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   hdmi@39 {
+   compatible = "adi,adv7511w";
+   reg = <0x39>;
+   interrupt-parent = <>;
+   interrupts = <0 IRQ_TYPE_LEVEL_LOW>;
+   clocks = <_clock>;
+   clock-names = "cec";
+   pd-gpios = < 24 GPIO_ACTIVE_HIGH>;
+
+   adi,input-depth = <8>;
+   adi,input-colorspace = "rgb";
+   adi,input-clock = "1x";
+   adi,input-style = <1>;
+   adi,input-justification = "evenly";
+
+   ports {
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   port@0 {
+   reg = <0>;
+   adv7511_in: endpoint {
+   remote-endpoint = 
<_out_rgb0>;
+   };
+   };
+
+   port@1 {
+   reg = <1>;
+   adv7511_out: endpoint {
+   remote-endpoint = <_con>;
+   };
+   };
+   };
+   };
+   };
 };
 
  {
@@ -70,45 +128,9 @@
 
  {
pinctrl-0 = <_pins>;
-   pinctrl-names = "default";
+   pinctrl-names = "i2c-hdmi";
 
-   status = "okay";
clock-frequency = <40>;
-
-   hdmi@39 {
-   compatible = "adi,adv7511w";
-   reg = <0x39>;
-   interrupt-parent = <>;
-   interrupts = <0 IRQ_TYPE_LEVEL_LOW>;
-   clocks = <_clock>;
-   clock-names = "cec";
-   pd-gpios = < 24 GPIO_ACTIVE_HIGH>;
-
-   adi,input-depth = <8>;
-   adi,input-colorspace = "rgb";
-   adi,input-clock = "1x";
-   adi,input-style = <1>;
-   adi,input-justification = "evenly";
-
-   ports {
-   #address-cells = <1>;
-   #size-cells = <0>;
-
-   port@0 {
-   reg = <0>;
-   adv7511_in: endpoint {
-   remote-endpoint = <_out_rgb0>;
-   };
-   };
-
-   port@1 {
-   reg = <1>;
-   adv7511_out: endpoint {
-   remote-endpoint = <_con>;
-   };
-   };
-   };
-   };
 };
 
  {
-- 
2.7.4

[PATCH vY 2/2] ARM: dts: iwg22d-sodimm: use demuxer for I2C5

2018-01-22 Thread Biju Das 
Create a separate bus for audiocodec related I2C5 and provide fallback to GPIO.

Based on work for r8a7794/Alt by Wolfram Sang: "ARM: dts: alt: use demuxer
for I2C1"

Signed-off-by: Biju Das 
Reviewed-by: Chris Paterson 
---
 arch/arm/boot/dts/r8a7745-iwg22d-sodimm.dts | 44 +
 1 file changed, 33 insertions(+), 11 deletions(-)

diff --git a/arch/arm/boot/dts/r8a7745-iwg22d-sodimm.dts 
b/arch/arm/boot/dts/r8a7745-iwg22d-sodimm.dts
index a4058f4..b2ea43f 100644
--- a/arch/arm/boot/dts/r8a7745-iwg22d-sodimm.dts
+++ b/arch/arm/boot/dts/r8a7745-iwg22d-sodimm.dts
@@ -42,6 +42,8 @@
ethernet0 = 
serial3 = 
serial5 = 
+   i2c9 = 
+   i2c11 = 
};
 
chosen {
@@ -55,6 +57,36 @@
clock-frequency = <2600>;
};
 
+   gpioi2c5: i2c-9 {
+   #address-cells = <1>;
+   #size-cells = <0>;
+   compatible = "i2c-gpio";
+   status = "disabled";
+   scl-gpios = < 14 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
+   sda-gpios = < 15 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
+   i2c-gpio,delay-us = <5>;
+   };
+
+   /*
+* A fallback to GPIO is provided for I2C5.
+*/
+   i2caudiocodec: i2c-11 {
+   compatible = "i2c-demux-pinctrl";
+   i2c-parent = <>, <>;
+   i2c-bus-name = "i2c-audiocodec";
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   sgtl5000: codec@a {
+   compatible = "fsl,sgtl5000";
+   #sound-dai-cells = <0>;
+   reg = <0x0a>;
+   clocks = <_clock>;
+   VDDA-supply = <_3p3v>;
+   VDDIO-supply = <_3p3v>;
+   };
+   };
+
rsnd_sgtl5000: sound {
compatible = "simple-audio-card";
simple-audio-card,format = "i2s";
@@ -126,19 +158,9 @@
 
  {
pinctrl-0 = <_pins>;
-   pinctrl-names = "default";
+   pinctrl-names = "i2c-audiocodec";
 
-   status = "okay";
clock-frequency = <40>;
-
-   sgtl5000: codec@a {
-   compatible = "fsl,sgtl5000";
-   #sound-dai-cells = <0>;
-   reg = <0x0a>;
-   clocks = <_clock>;
-   VDDA-supply = <_3p3v>;
-   VDDIO-supply = <_3p3v>;
-   };
 };
 
  {
-- 
2.7.4

Re: [PATCH v6 0/9] Renesas Capture Engine Unit (CEU) V4L2 driver

2018-01-22 Thread Hans Verkuil 
On 21/01/18 18:49, jacopo mondi wrote:
> Hi Hans,
> 
> On Fri, Jan 19, 2018 at 12:26:09PM +0100, Hans Verkuil wrote:
>> Hi Jacopo,
>>
>> On 01/16/18 22:44, Jacopo Mondi wrote:
>>> Hello,
>>>new version of CEU after Hans' review.
>>>
>>> Added his Acked-by to most patches and closed review comments.
>>> Running v4l2-compliance, I noticed a new failure introduced by the way I now
>>> calculate the plane sizes in set/try_fmt.
>>>
>>> This is the function used to update per-plane bytesperline and sizeimage:
>>>
>>> static void ceu_update_plane_sizes(struct v4l2_plane_pix_format *plane,
>>>unsigned int bpl, unsigned int szimage)
>>> {
>>> if (plane->bytesperline < bpl)
>>> plane->bytesperline = bpl;
>>> if (plane->sizeimage < szimage)
>>> plane->sizeimage = szimage;
>>> }
>>>
>>> I'm seeing a failure as v4l2-compliance requires buffers with both 
>>> bytesperline
>>> and sizeimage set to MAX_INT . Hans, is this expected from v4l2-compliance?
>>> How should I handle this, if that has to be handled by the single drivers?
>>
>> I commented on this in my review of patch 3/9.
> 
> Fixed thank you.
> 
>>
>>>
>>> Apart from that, here it is the output of v4l2-compliance, with the last 
>>> tests
>>> failing due to the above stated reason, and two errors in try/set format 
>>> due to
>>> the fact the driver is not setting ycbcr encoding after it receives an 
>>> invalid
>>
>> Which driver? The CEU driver or the sensor driver? I don't actually see where
>> it fails.
>>
> 
> Here it is:
> 
> fail: v4l2-test-formats.cpp(335): ycbcr_enc >= 0xff
> fail: v4l2-test-formats.cpp(451): 
> testColorspace(pix_mp.pixelformat, pix_mp.colorspace, pix_mp.ycbcr_enc, 
> pix_mp.quantization)
> fail: v4l2-test-formats.cpp(736): Video Capture Multiplanar 
> is valid, but TRY_FMT failed to return a format
> test VIDIOC_TRY_FMT: FAIL
> fail: v4l2-test-formats.cpp(335): ycbcr_enc >= 0xff
> fail: v4l2-test-formats.cpp(451): 
> testColorspace(pix_mp.pixelformat, pix_mp.colorspace, pix_mp.ycbcr_enc, 
> pix_mp.quantization)
> fail: v4l2-test-formats.cpp(996): Video Capture Multiplanar 
> is valid, but no S_FMT was implemented
> test VIDIOC_S_FMT: FAIL

Sorry, I was perhaps confusing. I meant that I couldn't see where in the code
ycbcr_enc was not overwritten by 0 (which should have happened). You will need
to debug a bit, I think. It could be a bug in the ceu driver or the sensor 
driver.

> 
> 
>>> format. I would set those, but I'm not sure what it the correct value and 
>>> not
>>> all mainline drivers do that.
>>
>> In any case, the default for ycbcr_enc, xfer_func and quantization is 0.
>>
> 
> Thanks again. I do expect to be the sensor driver to set ycbcr_enc and
> quantization, but from a very trivial grep on media/i2c/ I see only a
> few drivers taking care of them (adv7511 and adv7842). What about the
> others? I assume v4l2-compliance would not fail on them as it does on
> ov7670, but I don't see where ycbr_enc (and others) are managed.

In most cases these values are initialized to 0 (either a memset or by
initializing the struct), which is typically all you need for sensor
drivers. HDMI drivers are more complicated which is why you see explicit
handling of these fields only there.

> 
> Overall, with this addressed, the other issue I mentioned on patch
> [3/9] on readbuffers clarified and frameinterval handled for ov722x, I
> hope we're done with this series. Thanks again your continued effort
> in reviews and guidance.

My pleasure!

Regards,

Hans

Re: [PATCH v6 3/9] v4l: platform: Add Renesas CEU driver

2018-01-22 Thread Hans Verkuil 
On 21/01/18 18:29, jacopo mondi wrote:
> Hi Hans,
> 
> On Sun, Jan 21, 2018 at 11:23:12AM +0100, Hans Verkuil wrote:
>> On 21/01/18 11:21, Hans Verkuil wrote:
>>> On 21/01/18 10:53, jacopo mondi wrote:
 Hi Hans,

 On Fri, Jan 19, 2018 at 12:20:19PM +0100, Hans Verkuil wrote:
> static int ov7670_g_parm(struct v4l2_subdev *sd, struct v4l2_streamparm 
> *parms)
> {
> struct v4l2_captureparm *cp = >parm.capture;
> struct ov7670_info *info = to_state(sd);
>
> if (parms->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
> return -EINVAL;
>
> And parms->type is CAPTURE_MPLANE. Just drop this test from the ov7670 
> driver
> in the g/s_parm functions. It shouldn't test for that since a subdev 
> driver
> knows nothing about buffer types.
>

 I will drop that test in an additional patch part of next iteration of 
 this series,
>>>
>>> Replace g/s_parm by g/s_frame_interval. Consider g/s_parm for subdev 
>>> drivers as
>>> deprecated (I'm working on a patch series to replace all g/s_parm uses by
>>> g/s_frame_interval).
>>
>> Take a look here:
>>
>> https://git.linuxtv.org/hverkuil/media_tree.git/log/?h=parm
>>
>> You probably want to use the patch 'v4l2-common: add g/s_parm helper 
>> functions'
>> for the new ceu driver in your patch series. Feel free to add it.
> 
> Thanks, I have now re-based my series on top of your 'parm' branch,
> and now I have silenced those errors on bad frame interval.
> 
> CEU g/s_parm now look like this:
> 
> static int ceu_g_parm(struct file *file, void *fh, struct v4l2_streamparm *a)
> {
>   struct ceu_device *ceudev = video_drvdata(file);
>   int ret;
> 
>   ret = v4l2_g_parm(V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE,
> ceudev->sd->v4l2_sd, a);
>   if (ret)
>   return ret;
> 
>   a->parm.capture.readbuffers = 0;
> 
>   return 0;
> }
> 
> Very similar to what you've done on other platform drivers in this
> commit:
> https://git.linuxtv.org/hverkuil/media_tree.git/commit/?h=parm=a58956ef45cebaa5ce43a5f740fe04517b24a853
> 
> I have a question though (please bear with me a little more :)
> I had to manually set a->parm.capture.readbuffers to 0 to silence the 
> following
> error in v4l2_compliance (which I have now updated to the most recent
> remote HEAD):
> 
>  fail: v4l2-test-formats.cpp(1114): cap->readbuffers
> test VIDIOC_G/S_PARM: FAIL
> 
>   fail_on_test(cap->readbuffers > VIDEO_MAX_FRAME);
>   if (!(node->g_caps() & V4L2_CAP_READWRITE))
>   fail_on_test(cap->readbuffers);
>   else if (node->g_caps() & V4L2_CAP_STREAMING)
>   fail_on_test(!cap->readbuffers);
> 
> CEU does not support CAP_READWRITE, as it seems atmel-isc/isi do not, so
> v4l2-compliance wants to have readbuffers set to 0. I wonder why in
> the previously mentioned commit you didn't have to set readbuffers
> explicitly to 0 for atmel-isc/isi as I had to for CEU. Will v4l2-compliance
> fail if run on atmel-isc/isi with your commit, or am I missing something?

I've reworked the g/s_parm helper functions so they will now check for
the READWRITE capability and set readbuffers accordingly. I'll post a new
version later today.

Thanks for testing this, I missed that corner case.

Regards,

Hans

Re: [PATCH v6 3/9] v4l: platform: Add Renesas CEU driver

2018-01-22 Thread Hans Verkuil 
On 22/01/18 01:52, Laurent Pinchart wrote:
> Hi Hans,
> 
> On Friday, 19 January 2018 14:25:39 EET Hans Verkuil wrote:
>> On 01/19/18 13:20, Laurent Pinchart wrote:
>>> On Friday, 19 January 2018 13:20:19 EET Hans Verkuil wrote:
 On 01/16/18 22:44, Jacopo Mondi wrote:
> Add driver for Renesas Capture Engine Unit (CEU).
>
> The CEU interface supports capturing 'data' (YUV422) and 'images'
> (NV[12|21|16|61]).
>
> This driver aims to replace the soc_camera-based sh_mobile_ceu one.
>
> Tested with ov7670 camera sensor, providing YUYV_2X8 data on Renesas RZ
> platform GR-Peach.
>
> Tested with ov7725 camera sensor on SH4 platform Migo-R.
>
> Signed-off-by: Jacopo Mondi 
> Reviewed-by: Laurent Pinchart 
> ---
>
>  drivers/media/platform/Kconfig   |9 +
>  drivers/media/platform/Makefile  |1 +
>  drivers/media/platform/renesas-ceu.c | 1659 +++
>  3 files changed, 1669 insertions(+)
>  create mode 100644 drivers/media/platform/renesas-ceu.c
>>>
>>> [snip]
>>>
> diff --git a/drivers/media/platform/renesas-ceu.c
> b/drivers/media/platform/renesas-ceu.c new file mode 100644
> index 000..1b8f0ef
> --- /dev/null
> +++ b/drivers/media/platform/renesas-ceu.c
>>>
>>> [snip]
>>>
> +static void ceu_update_plane_sizes(struct v4l2_plane_pix_format *plane,
> +unsigned int bpl, unsigned int szimage)
> +{
> + if (plane->bytesperline < bpl)
> + plane->bytesperline = bpl;
> + if (plane->sizeimage < szimage)
> + plane->sizeimage = szimage;

 As mentioned in your cover letter, you do need to check for invalid
 bytesperline values. The v4l2-compliance test is to see what happens
 when userspace gives insane values, so yes, drivers need to be able
 to handle that.
>>>
>>> What limit would you set, what is an acceptable large value versus an
>>> invalid large value ? I think we should have rules for this at the API
>>> level (or at least, if not part of the API, rules that are consistent
>>> across drivers).
>>
>> I would expect this to be the max of what the hardware can support. If
>> that's really high, then this can be, say, 4 times the width.
>>
>> Note that there are very few drivers that can handle a user-specified
>> stride.
> 
> But that's no reason not to handle it here if the hardware permits, right ? 
> :-)

Certainly. But it is the reason why it's hard to find example code in
existing drivers.

 plane->sizeimage is set by the driver, so drop the 'if' before the
 assignment.
>>>
>>> I don't think that's correct. Userspace should be able to control padding
>>> lines at the end of the image, the same way it controls padding pixels at
>>> the end of lines.
>>
>> If userspace wants larger buffers, then it should use VIDIOC_CREATE_BUFS.
>>
>> sizeimage is exclusively set by the driver, applications rely on that.
> 
> The API documentation is pretty confusing about this.
> 
> In pixfmt-v4l2.rst, the field in the v4l2_pix_format structure is documented 
> as
> 
>   - Size in bytes of the buffer to hold a complete image, set by the
> driver. Usually this is ``bytesperline`` times ``height``. When
> the image consists of variable length compressed data this is the
> maximum number of bytes required to hold an image.
> 
> Then in pixfmt-v4l2-mplane.rst, the field in the v4l2_plane_pix_format 
> structure is documented as
> 
>   - Maximum size in bytes required for image data in this plane.

This should contain the same text as in pixfmt-v4l2.rst.

> 
> Finally, in vidioc-create-bufs.rst, we have
> 
> The buffers created by this ioctl will have as minimum size the size
> defined by the ``format.pix.sizeimage`` field (or the corresponding
> fields for other format types). Usually if the ``format.pix.sizeimage``
> field is less than the minimum required for the given format, then an
> error will be returned since drivers will typically not allow this. If
> it is larger, then the value will be used as-is. In other words, the
> driver may reject the requested size, but if it is accepted the driver
> will use it unchanged.
> 
> The VIDIOC_CREATE_BUFS documentation contradicts the v4l2_pix_format 
> documentation, as the multiplane case doesn't state anything about who sets 
> the sizeimage field. We should clarify the documentation.

The pixfmt-v4l2-mplane.rst is the one that's incomplete. I noticed the same
thing when I looked at it.

> 
> +}
>>>
>>> [snip]
>>>
> +static const struct v4l2_ioctl_ops ceu_ioctl_ops = {
> + .vidioc_querycap= ceu_querycap,
> +
> + .vidioc_enum_fmt_vid_cap_mplane = ceu_enum_fmt_vid_cap,
> + .vidioc_try_fmt_vid_cap_mplane  = ceu_try_fmt_vid_cap,
> + .vidioc_s_fmt_vid_cap_mplane= ceu_s_fmt_vid_cap,
> +

Re: [RFC PATCH v2 0/1] of: easier debugging for node life cycle issues

2018-01-22 Thread Frank Rowand 
On 01/21/18 06:31, Wolfram Sang wrote:
> I got a bug report for a DT node refcounting problem in the I2C subsystem. 
> This
> patch was a huge help in validating the bug report and the proposed solution.
> So, I thought I bring it to attention again. Thanks Tyrel, for the initial
> work!
> 
> Note that I did not test the dynamic updates, only of_node_{get|put} so far. I
> read that Tyrel checked dynamic updates extensively with this patch. And since
> DT overlays are also used within our Renesas dev team, this will help there, 
> as
> well.
> 
> Tested on a Renesas Salvator-XS board (R-Car H3).
> 
> Changes since RFC v1:
>   * rebased to v4.15-rc8
>   * fixed commit abbrev and one of the sysfs paths in commit desc
>   * removed trailing space and fixed pointer declaration in code
> 
> I consider all the remaining checkpatch issues irrelevant for this patch.
> 
> So what about applying it?
> 
> Kind regards,
> 
>Wolfram
> 
> 
> Tyrel Datwyler (1):
>   of: introduce event tracepoints for dynamic device_node lifecyle
> 
>  drivers/of/dynamic.c  | 32 ++--
>  include/trace/events/of.h | 93 
> +++
>  2 files changed, 105 insertions(+), 20 deletions(-)
>  create mode 100644 include/trace/events/of.h
> 

Please go back and read the thread for version 1.  Simply resubmitting a
forward port is ignoring that whole conversation.

There is a lot of good info in that thread.  I certainly learned stuff in it.

Thanks,

Frank