Re: [PATCH v4] media: spi: Add support for LMH0395

2015-01-09 Thread Jean-Michel Hautbois
Hi Laurent,

Thanks for the review.

2014-11-03 14:13 GMT+01:00 Laurent Pinchart :
> Hi Jean-Michel,
>
> Thank you for the patch.
>
> On Wednesday 10 September 2014 11:43:54 Jean-Michel Hautbois wrote:
>> This device is a SPI based device from TI.
>> It is a 3 Gbps HD/SD SDI Dual Output Low Power
>> Extended Reach Adaptive Cable Equalizer.
>>
>> LMH0395 enables the use of up to two outputs.
>> These can be configured using DT.
>>
>> Controls should be accessible from userspace too.
>> This will have to be done later.
>>
>> v2: Add DT support
>> v3: Change the bit set/clear in output_type as disabled means 'set the bit'
>> v4: Clearer description of endpoints usage in Doc, and some init changes.
>> Add a dependency on OF and don't test CONFIG_OF anymore.
>>
>> Signed-off-by: Jean-Michel Hautbois 
>> ---
>>  .../devicetree/bindings/media/spi/lmh0395.txt  |  48 +++
>>  MAINTAINERS|   6 +
>>  drivers/media/spi/Kconfig  |  14 +
>>  drivers/media/spi/Makefile |   1 +
>>  drivers/media/spi/lmh0395.c| 354 ++
>>  5 files changed, 423 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/media/spi/lmh0395.txt
>>  create mode 100644 drivers/media/spi/Kconfig
>>  create mode 100644 drivers/media/spi/Makefile
>>  create mode 100644 drivers/media/spi/lmh0395.c
>>
>> diff --git a/Documentation/devicetree/bindings/media/spi/lmh0395.txt
>> b/Documentation/devicetree/bindings/media/spi/lmh0395.txt new file mode
>> 100644
>> index 000..7cc0026
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/media/spi/lmh0395.txt
>> @@ -0,0 +1,48 @@
>> +* Texas Instruments lmh0395 3G HD/SD SDI equalizer
>> +
>> +The LMH0395 3 Gbps HD/SD SDI Dual Output Low Power Extended Reach Adaptive
>> +Cable Equalizer is designed to equalize data transmitted over cable (or any
>> +media with similar dispersive loss characteristics).
>> +The equalizer operates over a wide range of data rates from 125 Mbps to
>> 2.97 Gbps
>> +and supports SMPTE 424M, SMPTE 292M, SMPTE344M, SMPTE 259M, and DVB-ASI
>> standards.
>
> That's copied verbatim from the datasheet and not very relevant from a DT
> bindings point of view. I would rather explain what the chip does. Something
> like
>
> "The LMH0395 is an SDI equalizer designed to extend the reach of SDI signals
> transmitted over cable by equalizing the input signal and generating clean
> outputs. It has one differential input and two differential output that can be
> independently controlled."

OK, applied.

>> +
>> +Required Properties :
>> +- compatible: Must be "ti,lmh0395"
>> +
>> +The device node must contain one 'port' child node per device input and
>> output
>> +port, in accordance with the video interface bindings defined in
>> +Documentation/devicetree/bindings/media/video-interfaces.txt. The port
>> nodes
>> +are numbered as follows.
>> +
>> +  Port   LMH0395
>> +
>> +  SDI input  0
>> +  SDI output 1,2
>
> While there shouldn't be much doubt about SDO0 corresponding to port 1 and
> SDO1 to port 2, I'd like that to be mentioned explicitly.
>
> The device node must contain one 'port' child node per device input and output
> port, in accordance with the video interface bindings defined in
> Documentation/devicetree/bindings/media/video-interfaces.txt.
>
> The LMH0395 has three ports numbered as follows.
>
> DescriptionNumber
> 
> SDI (SDI input) 0
> SDO0 (SDI output 0) 1
> SDO1 (SDI output 1) 2

OK.

>> +Example:
>> +
>> +ecspi@0201 {
>> + ...
>> + ...
>> +
>> + lmh0395@1 {
>> + compatible = "ti,lmh0395";
>> + reg = <1>;
>> + spi-max-frequency = <2000>;
>> + ports {
>
> You need to add
>
> #address-cells = <1>;
> #size-cells = <0>;
>
> here.
>

Of course, applied.

>> + port@0 {
>> + reg = <0>;
>> + sdi0_in: endpoint {};
>> + };
>> + port@1 {
>> + reg = <1>;
>> + sdi0_out0: endpoint {};
>> + };
>> + port@2 {
>> + reg = <2>;
>> + /* endpoint not specified, disable output */
>> + };
>> + };
>> + };
>> + ...
>> +};
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index cf24bb5..ca42b9e 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -9141,6 +9141,12 @@ S: Maintained
>>  F:   sound/soc/codecs/lm49453*
>>  F:   sound/soc/codecs/isabelle*
>>
>> +TI LMH0395 DRIVER
>> +M:   Jean-Michel Hautbois 
>> +L:   linux-me...@vger.kernel.org
>> +S:   Maintained
>> +F:   drivers/media/spi/lmh0395*
>> +

Re: [PATCH v4] media: spi: Add support for LMH0395

2015-01-09 Thread Jean-Michel Hautbois
Hi Laurent,

Thanks for the review.

2014-11-03 14:13 GMT+01:00 Laurent Pinchart laurent.pinch...@ideasonboard.com:
 Hi Jean-Michel,

 Thank you for the patch.

 On Wednesday 10 September 2014 11:43:54 Jean-Michel Hautbois wrote:
 This device is a SPI based device from TI.
 It is a 3 Gbps HD/SD SDI Dual Output Low Power
 Extended Reach Adaptive Cable Equalizer.

 LMH0395 enables the use of up to two outputs.
 These can be configured using DT.

 Controls should be accessible from userspace too.
 This will have to be done later.

 v2: Add DT support
 v3: Change the bit set/clear in output_type as disabled means 'set the bit'
 v4: Clearer description of endpoints usage in Doc, and some init changes.
 Add a dependency on OF and don't test CONFIG_OF anymore.

 Signed-off-by: Jean-Michel Hautbois jean-michel.hautb...@vodalys.com
 ---
  .../devicetree/bindings/media/spi/lmh0395.txt  |  48 +++
  MAINTAINERS|   6 +
  drivers/media/spi/Kconfig  |  14 +
  drivers/media/spi/Makefile |   1 +
  drivers/media/spi/lmh0395.c| 354 ++
  5 files changed, 423 insertions(+)
  create mode 100644 Documentation/devicetree/bindings/media/spi/lmh0395.txt
  create mode 100644 drivers/media/spi/Kconfig
  create mode 100644 drivers/media/spi/Makefile
  create mode 100644 drivers/media/spi/lmh0395.c

 diff --git a/Documentation/devicetree/bindings/media/spi/lmh0395.txt
 b/Documentation/devicetree/bindings/media/spi/lmh0395.txt new file mode
 100644
 index 000..7cc0026
 --- /dev/null
 +++ b/Documentation/devicetree/bindings/media/spi/lmh0395.txt
 @@ -0,0 +1,48 @@
 +* Texas Instruments lmh0395 3G HD/SD SDI equalizer
 +
 +The LMH0395 3 Gbps HD/SD SDI Dual Output Low Power Extended Reach Adaptive
 +Cable Equalizer is designed to equalize data transmitted over cable (or any
 +media with similar dispersive loss characteristics).
 +The equalizer operates over a wide range of data rates from 125 Mbps to
 2.97 Gbps
 +and supports SMPTE 424M, SMPTE 292M, SMPTE344M, SMPTE 259M, and DVB-ASI
 standards.

 That's copied verbatim from the datasheet and not very relevant from a DT
 bindings point of view. I would rather explain what the chip does. Something
 like

 The LMH0395 is an SDI equalizer designed to extend the reach of SDI signals
 transmitted over cable by equalizing the input signal and generating clean
 outputs. It has one differential input and two differential output that can be
 independently controlled.

OK, applied.

 +
 +Required Properties :
 +- compatible: Must be ti,lmh0395
 +
 +The device node must contain one 'port' child node per device input and
 output
 +port, in accordance with the video interface bindings defined in
 +Documentation/devicetree/bindings/media/video-interfaces.txt. The port
 nodes
 +are numbered as follows.
 +
 +  Port   LMH0395
 +
 +  SDI input  0
 +  SDI output 1,2

 While there shouldn't be much doubt about SDO0 corresponding to port 1 and
 SDO1 to port 2, I'd like that to be mentioned explicitly.

 The device node must contain one 'port' child node per device input and output
 port, in accordance with the video interface bindings defined in
 Documentation/devicetree/bindings/media/video-interfaces.txt.

 The LMH0395 has three ports numbered as follows.

 DescriptionNumber
 
 SDI (SDI input) 0
 SDO0 (SDI output 0) 1
 SDO1 (SDI output 1) 2

OK.

 +Example:
 +
 +ecspi@0201 {
 + ...
 + ...
 +
 + lmh0395@1 {
 + compatible = ti,lmh0395;
 + reg = 1;
 + spi-max-frequency = 2000;
 + ports {

 You need to add

 #address-cells = 1;
 #size-cells = 0;

 here.


Of course, applied.

 + port@0 {
 + reg = 0;
 + sdi0_in: endpoint {};
 + };
 + port@1 {
 + reg = 1;
 + sdi0_out0: endpoint {};
 + };
 + port@2 {
 + reg = 2;
 + /* endpoint not specified, disable output */
 + };
 + };
 + };
 + ...
 +};
 diff --git a/MAINTAINERS b/MAINTAINERS
 index cf24bb5..ca42b9e 100644
 --- a/MAINTAINERS
 +++ b/MAINTAINERS
 @@ -9141,6 +9141,12 @@ S: Maintained
  F:   sound/soc/codecs/lm49453*
  F:   sound/soc/codecs/isabelle*

 +TI LMH0395 DRIVER
 +M:   Jean-Michel Hautbois jean-michel.hautb...@vodalys.com
 +L:   linux-me...@vger.kernel.org
 +S:   Maintained
 +F:   drivers/media/spi/lmh0395*
 +
  TI LP855x BACKLIGHT DRIVER
  M:   Milo Kim milo@ti.com
  S:   Maintained
 diff --git a/drivers/media/spi/Kconfig b/drivers/media/spi/Kconfig
 new file mode 

Re: [PATCH v4] media: spi: Add support for LMH0395

2014-11-03 Thread Hans Verkuil
On 11/03/2014 02:42 PM, Laurent Pinchart wrote:
> On Monday 03 November 2014 14:36:26 Hans Verkuil wrote:
>> On 11/03/2014 02:13 PM, Laurent Pinchart wrote:
>>> Hi Jean-Michel,
>>>
>>> Thank you for the patch.
>>
>>> On Wednesday 10 September 2014 11:43:54 Jean-Michel Hautbois wrote:
>> 
>>
 +static int lmh0395_s_routing(struct v4l2_subdev *sd, u32 input, u32
 output, +  u32 config)
 +{
 +  struct lmh0395_state *state = to_state(sd);
 +  int err = 0;
 +
 +  if (state->output_type != output)
 +  err = lmh0395_set_output_type(sd, output);
 +
 +  return err;

>>> if (state->output_type == output)
>>> return 0;
>>> 
>>> return lmh0395_set_output_type(sd, output);
>>>
>>> You can then get rid of the err variable.
>>>
>>> I don't really like this implementation though, the output argument is
>>> device- specific, you thus require explicit knowledge of the LMH0395 in
>>> your bridge driver.
>>
>> Well, that's the way s_routing is defined. It's the bridge driver's job to
>> translate between V4L2 inputs/outputs and low-level routing information.
>>
>>> I'm not sure what the config argument is used for, but naively I would
>>> have
>>
>> config is normally not used. There are one or two drivers that need it for
>> additional routing configuration, but it's rare.
> 
> OK.
> 
>>> used it to tell whether to enable (1) or disable (0) the route from input
>>> to output. The input value should then always be 0, and the output value
>>> can be 1 or 2. Another option would be to use the new S_ROUTING userspace
>>> ioctl I've proposed in
>>> http://git.linuxtv.org/cgit.cgi/pinchartl/media.git/commit/?h=xilinx-wip
>>> d=fc094c354338861673464ed4b8fa198089fe7bf0.
>>>
>>> Hans, could you comment on that ?
>>
>> Well, first of all your proposed API isn't merged yet, or even posted on the
>> mailinglist, so it's a bit unfair to require someone else to use it :-)
> 
> It's not a requirement, just a proposal :-) I can send a patch series if 
> there's enough interest for using that API.
> 
>> Also, while I do agree with your proposed new API I am a lot less
>> enthusiastic about creating yet another duplicate pad op for an existing
>> audio/video routing op.
>>
>> The problem is that existing drivers are never updated for the new op and
>> you are stuck with competing internal APIs. Not nice at all.
> 
> Agreed, it's not nice, feel free to propose a better solution :-)
> 
>> Bottom line is that this driver uses s_routing like any other driver
>> currently in the kernel, so I have no problem with that.
> 
> I do :-)
> 
> The bridge to subdev dependency is fine for PCI or USB devices where the 
> hardware topology is known to the driver. It isn't for composite embedded 
> devices, as we don't want to teach all bridges about all subdevs.
> 

I agree, and your proposal is a nice solution. But whoever want to get this
merged *will* have to take a good look at the existing g/s_routing ops and
how they can be converted to the new one. I didn't block that when similar
duplicate ops were added in the past and I am increasingly sorry I didn't do
that. It's creating incompatible subdevs, where the whole point of the subdev
API was to avoid that.

Regards,

Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v4] media: spi: Add support for LMH0395

2014-11-03 Thread Laurent Pinchart
On Monday 03 November 2014 14:36:26 Hans Verkuil wrote:
> On 11/03/2014 02:13 PM, Laurent Pinchart wrote:
> > Hi Jean-Michel,
> > 
> > Thank you for the patch.
> 
> > On Wednesday 10 September 2014 11:43:54 Jean-Michel Hautbois wrote:
> 
> 
> >> +static int lmh0395_s_routing(struct v4l2_subdev *sd, u32 input, u32
> >> output, +  u32 config)
> >> +{
> >> +  struct lmh0395_state *state = to_state(sd);
> >> +  int err = 0;
> >> +
> >> +  if (state->output_type != output)
> >> +  err = lmh0395_set_output_type(sd, output);
> >> +
> >> +  return err;
> >> 
> > if (state->output_type == output)
> > return 0;
> > 
> > return lmh0395_set_output_type(sd, output);
> > 
> > You can then get rid of the err variable.
> > 
> > I don't really like this implementation though, the output argument is
> > device- specific, you thus require explicit knowledge of the LMH0395 in
> > your bridge driver.
> 
> Well, that's the way s_routing is defined. It's the bridge driver's job to
> translate between V4L2 inputs/outputs and low-level routing information.
> 
> > I'm not sure what the config argument is used for, but naively I would
> > have
> 
> config is normally not used. There are one or two drivers that need it for
> additional routing configuration, but it's rare.

OK.

> > used it to tell whether to enable (1) or disable (0) the route from input
> > to output. The input value should then always be 0, and the output value
> > can be 1 or 2. Another option would be to use the new S_ROUTING userspace
> > ioctl I've proposed in
> > http://git.linuxtv.org/cgit.cgi/pinchartl/media.git/commit/?h=xilinx-wip
> > d=fc094c354338861673464ed4b8fa198089fe7bf0.
> > 
> > Hans, could you comment on that ?
> 
> Well, first of all your proposed API isn't merged yet, or even posted on the
> mailinglist, so it's a bit unfair to require someone else to use it :-)

It's not a requirement, just a proposal :-) I can send a patch series if 
there's enough interest for using that API.

> Also, while I do agree with your proposed new API I am a lot less
> enthusiastic about creating yet another duplicate pad op for an existing
> audio/video routing op.
> 
> The problem is that existing drivers are never updated for the new op and
> you are stuck with competing internal APIs. Not nice at all.

Agreed, it's not nice, feel free to propose a better solution :-)

> Bottom line is that this driver uses s_routing like any other driver
> currently in the kernel, so I have no problem with that.

I do :-)

The bridge to subdev dependency is fine for PCI or USB devices where the 
hardware topology is known to the driver. It isn't for composite embedded 
devices, as we don't want to teach all bridges about all subdevs.

-- 
Regards,

Laurent Pinchart

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v4] media: spi: Add support for LMH0395

2014-11-03 Thread Hans Verkuil
On 11/03/2014 02:13 PM, Laurent Pinchart wrote:
> Hi Jean-Michel,
> 
> Thank you for the patch.
> 
> On Wednesday 10 September 2014 11:43:54 Jean-Michel Hautbois wrote:



> 
>> +static int lmh0395_s_routing(struct v4l2_subdev *sd, u32 input, u32 output,
>> +u32 config)
>> +{
>> +struct lmh0395_state *state = to_state(sd);
>> +int err = 0;
>> +
>> +if (state->output_type != output)
>> +err = lmh0395_set_output_type(sd, output);
>> +
>> +return err;
> 
> if (state->output_type == output)
> return 0;
> 
> return lmh0395_set_output_type(sd, output);
> 
> You can then get rid of the err variable.
> 
> I don't really like this implementation though, the output argument is device-
> specific, you thus require explicit knowledge of the LMH0395 in your bridge 
> driver.

Well, that's the way s_routing is defined. It's the bridge driver's job to
translate between V4L2 inputs/outputs and low-level routing information.

> 
> I'm not sure what the config argument is used for, but naively I would have 

config is normally not used. There are one or two drivers that need it for
additional routing configuration, but it's rare.

> used it to tell whether to enable (1) or disable (0) the route from input to 
> output. The input value should then always be 0, and the output value can be 
> 1 
> or 2. Another option would be to use the new S_ROUTING userspace ioctl I've 
> proposed in 
> http://git.linuxtv.org/cgit.cgi/pinchartl/media.git/commit/?h=xilinx-wip=fc094c354338861673464ed4b8fa198089fe7bf0.
> 
> Hans, could you comment on that ?

Well, first of all your proposed API isn't merged yet, or even posted on the
mailinglist, so it's a bit unfair to require someone else to use it :-)

Also, while I do agree with your proposed new API I am a lot less enthusiastic
about creating yet another duplicate pad op for an existing audio/video routing
op.

The problem is that existing drivers are never updated for the new op and you 
are
stuck with competing internal APIs. Not nice at all.

Bottom line is that this driver uses s_routing like any other driver currently
in the kernel, so I have no problem with that.

Regards,

Hans

> 
>> +}

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v4] media: spi: Add support for LMH0395

2014-11-03 Thread Laurent Pinchart
Hi Jean-Michel,

Thank you for the patch.

On Wednesday 10 September 2014 11:43:54 Jean-Michel Hautbois wrote:
> This device is a SPI based device from TI.
> It is a 3 Gbps HD/SD SDI Dual Output Low Power
> Extended Reach Adaptive Cable Equalizer.
> 
> LMH0395 enables the use of up to two outputs.
> These can be configured using DT.
> 
> Controls should be accessible from userspace too.
> This will have to be done later.
> 
> v2: Add DT support
> v3: Change the bit set/clear in output_type as disabled means 'set the bit'
> v4: Clearer description of endpoints usage in Doc, and some init changes.
> Add a dependency on OF and don't test CONFIG_OF anymore.
> 
> Signed-off-by: Jean-Michel Hautbois 
> ---
>  .../devicetree/bindings/media/spi/lmh0395.txt  |  48 +++
>  MAINTAINERS|   6 +
>  drivers/media/spi/Kconfig  |  14 +
>  drivers/media/spi/Makefile |   1 +
>  drivers/media/spi/lmh0395.c| 354 ++
>  5 files changed, 423 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/spi/lmh0395.txt
>  create mode 100644 drivers/media/spi/Kconfig
>  create mode 100644 drivers/media/spi/Makefile
>  create mode 100644 drivers/media/spi/lmh0395.c
> 
> diff --git a/Documentation/devicetree/bindings/media/spi/lmh0395.txt
> b/Documentation/devicetree/bindings/media/spi/lmh0395.txt new file mode
> 100644
> index 000..7cc0026
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/spi/lmh0395.txt
> @@ -0,0 +1,48 @@
> +* Texas Instruments lmh0395 3G HD/SD SDI equalizer
> +
> +The LMH0395 3 Gbps HD/SD SDI Dual Output Low Power Extended Reach Adaptive
> +Cable Equalizer is designed to equalize data transmitted over cable (or any
> +media with similar dispersive loss characteristics).
> +The equalizer operates over a wide range of data rates from 125 Mbps to
> 2.97 Gbps
> +and supports SMPTE 424M, SMPTE 292M, SMPTE344M, SMPTE 259M, and DVB-ASI
> standards.

That's copied verbatim from the datasheet and not very relevant from a DT 
bindings point of view. I would rather explain what the chip does. Something 
like

"The LMH0395 is an SDI equalizer designed to extend the reach of SDI signals 
transmitted over cable by equalizing the input signal and generating clean 
outputs. It has one differential input and two differential output that can be 
independently controlled."

> +
> +Required Properties :
> +- compatible: Must be "ti,lmh0395"
> +
> +The device node must contain one 'port' child node per device input and
> output
> +port, in accordance with the video interface bindings defined in
> +Documentation/devicetree/bindings/media/video-interfaces.txt. The port
> nodes
> +are numbered as follows.
> +
> +  Port   LMH0395
> +
> +  SDI input  0
> +  SDI output 1,2

While there shouldn't be much doubt about SDO0 corresponding to port 1 and 
SDO1 to port 2, I'd like that to be mentioned explicitly.

The device node must contain one 'port' child node per device input and output
port, in accordance with the video interface bindings defined in 
Documentation/devicetree/bindings/media/video-interfaces.txt.

The LMH0395 has three ports numbered as follows.

DescriptionNumber

SDI (SDI input) 0
SDO0 (SDI output 0) 1
SDO1 (SDI output 1) 2

> +Example:
> +
> +ecspi@0201 {
> + ...
> + ...
> +
> + lmh0395@1 {
> + compatible = "ti,lmh0395";
> + reg = <1>;
> + spi-max-frequency = <2000>;
> + ports {

You need to add

#address-cells = <1>;
#size-cells = <0>;

here.

> + port@0 {
> + reg = <0>;
> + sdi0_in: endpoint {};
> + };
> + port@1 {
> + reg = <1>;
> + sdi0_out0: endpoint {};
> + };
> + port@2 {
> + reg = <2>;
> + /* endpoint not specified, disable output */
> + };
> + };
> + };
> + ...
> +};
> diff --git a/MAINTAINERS b/MAINTAINERS
> index cf24bb5..ca42b9e 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -9141,6 +9141,12 @@ S: Maintained
>  F:   sound/soc/codecs/lm49453*
>  F:   sound/soc/codecs/isabelle*
> 
> +TI LMH0395 DRIVER
> +M:   Jean-Michel Hautbois 
> +L:   linux-me...@vger.kernel.org
> +S:   Maintained
> +F:   drivers/media/spi/lmh0395*
> +
>  TI LP855x BACKLIGHT DRIVER
>  M:   Milo Kim 
>  S:   Maintained
> diff --git a/drivers/media/spi/Kconfig b/drivers/media/spi/Kconfig
> new file mode 100644
> index 000..bcb1ab1
> --- /dev/null
> +++ b/drivers/media/spi/Kconfig
> @@ -0,0 +1,14 @@
> +if VIDEO_V4L2
> +

Re: [PATCH v4] media: spi: Add support for LMH0395

2014-11-03 Thread Laurent Pinchart
Hi Jean-Michel,

Thank you for the patch.

On Wednesday 10 September 2014 11:43:54 Jean-Michel Hautbois wrote:
 This device is a SPI based device from TI.
 It is a 3 Gbps HD/SD SDI Dual Output Low Power
 Extended Reach Adaptive Cable Equalizer.
 
 LMH0395 enables the use of up to two outputs.
 These can be configured using DT.
 
 Controls should be accessible from userspace too.
 This will have to be done later.
 
 v2: Add DT support
 v3: Change the bit set/clear in output_type as disabled means 'set the bit'
 v4: Clearer description of endpoints usage in Doc, and some init changes.
 Add a dependency on OF and don't test CONFIG_OF anymore.
 
 Signed-off-by: Jean-Michel Hautbois jean-michel.hautb...@vodalys.com
 ---
  .../devicetree/bindings/media/spi/lmh0395.txt  |  48 +++
  MAINTAINERS|   6 +
  drivers/media/spi/Kconfig  |  14 +
  drivers/media/spi/Makefile |   1 +
  drivers/media/spi/lmh0395.c| 354 ++
  5 files changed, 423 insertions(+)
  create mode 100644 Documentation/devicetree/bindings/media/spi/lmh0395.txt
  create mode 100644 drivers/media/spi/Kconfig
  create mode 100644 drivers/media/spi/Makefile
  create mode 100644 drivers/media/spi/lmh0395.c
 
 diff --git a/Documentation/devicetree/bindings/media/spi/lmh0395.txt
 b/Documentation/devicetree/bindings/media/spi/lmh0395.txt new file mode
 100644
 index 000..7cc0026
 --- /dev/null
 +++ b/Documentation/devicetree/bindings/media/spi/lmh0395.txt
 @@ -0,0 +1,48 @@
 +* Texas Instruments lmh0395 3G HD/SD SDI equalizer
 +
 +The LMH0395 3 Gbps HD/SD SDI Dual Output Low Power Extended Reach Adaptive
 +Cable Equalizer is designed to equalize data transmitted over cable (or any
 +media with similar dispersive loss characteristics).
 +The equalizer operates over a wide range of data rates from 125 Mbps to
 2.97 Gbps
 +and supports SMPTE 424M, SMPTE 292M, SMPTE344M, SMPTE 259M, and DVB-ASI
 standards.

That's copied verbatim from the datasheet and not very relevant from a DT 
bindings point of view. I would rather explain what the chip does. Something 
like

The LMH0395 is an SDI equalizer designed to extend the reach of SDI signals 
transmitted over cable by equalizing the input signal and generating clean 
outputs. It has one differential input and two differential output that can be 
independently controlled.

 +
 +Required Properties :
 +- compatible: Must be ti,lmh0395
 +
 +The device node must contain one 'port' child node per device input and
 output
 +port, in accordance with the video interface bindings defined in
 +Documentation/devicetree/bindings/media/video-interfaces.txt. The port
 nodes
 +are numbered as follows.
 +
 +  Port   LMH0395
 +
 +  SDI input  0
 +  SDI output 1,2

While there shouldn't be much doubt about SDO0 corresponding to port 1 and 
SDO1 to port 2, I'd like that to be mentioned explicitly.

The device node must contain one 'port' child node per device input and output
port, in accordance with the video interface bindings defined in 
Documentation/devicetree/bindings/media/video-interfaces.txt.

The LMH0395 has three ports numbered as follows.

DescriptionNumber

SDI (SDI input) 0
SDO0 (SDI output 0) 1
SDO1 (SDI output 1) 2

 +Example:
 +
 +ecspi@0201 {
 + ...
 + ...
 +
 + lmh0395@1 {
 + compatible = ti,lmh0395;
 + reg = 1;
 + spi-max-frequency = 2000;
 + ports {

You need to add

#address-cells = 1;
#size-cells = 0;

here.

 + port@0 {
 + reg = 0;
 + sdi0_in: endpoint {};
 + };
 + port@1 {
 + reg = 1;
 + sdi0_out0: endpoint {};
 + };
 + port@2 {
 + reg = 2;
 + /* endpoint not specified, disable output */
 + };
 + };
 + };
 + ...
 +};
 diff --git a/MAINTAINERS b/MAINTAINERS
 index cf24bb5..ca42b9e 100644
 --- a/MAINTAINERS
 +++ b/MAINTAINERS
 @@ -9141,6 +9141,12 @@ S: Maintained
  F:   sound/soc/codecs/lm49453*
  F:   sound/soc/codecs/isabelle*
 
 +TI LMH0395 DRIVER
 +M:   Jean-Michel Hautbois jean-michel.hautb...@vodalys.com
 +L:   linux-me...@vger.kernel.org
 +S:   Maintained
 +F:   drivers/media/spi/lmh0395*
 +
  TI LP855x BACKLIGHT DRIVER
  M:   Milo Kim milo@ti.com
  S:   Maintained
 diff --git a/drivers/media/spi/Kconfig b/drivers/media/spi/Kconfig
 new file mode 100644
 index 000..bcb1ab1
 --- /dev/null
 +++ b/drivers/media/spi/Kconfig
 @@ -0,0 +1,14 @@
 +if VIDEO_V4L2
 +
 +config VIDEO_LMH0395
 + tristate LMH0395 

Re: [PATCH v4] media: spi: Add support for LMH0395

2014-11-03 Thread Hans Verkuil
On 11/03/2014 02:13 PM, Laurent Pinchart wrote:
 Hi Jean-Michel,
 
 Thank you for the patch.
 
 On Wednesday 10 September 2014 11:43:54 Jean-Michel Hautbois wrote:

snip

 
 +static int lmh0395_s_routing(struct v4l2_subdev *sd, u32 input, u32 output,
 +u32 config)
 +{
 +struct lmh0395_state *state = to_state(sd);
 +int err = 0;
 +
 +if (state-output_type != output)
 +err = lmh0395_set_output_type(sd, output);
 +
 +return err;
 
 if (state-output_type == output)
 return 0;
 
 return lmh0395_set_output_type(sd, output);
 
 You can then get rid of the err variable.
 
 I don't really like this implementation though, the output argument is device-
 specific, you thus require explicit knowledge of the LMH0395 in your bridge 
 driver.

Well, that's the way s_routing is defined. It's the bridge driver's job to
translate between V4L2 inputs/outputs and low-level routing information.

 
 I'm not sure what the config argument is used for, but naively I would have 

config is normally not used. There are one or two drivers that need it for
additional routing configuration, but it's rare.

 used it to tell whether to enable (1) or disable (0) the route from input to 
 output. The input value should then always be 0, and the output value can be 
 1 
 or 2. Another option would be to use the new S_ROUTING userspace ioctl I've 
 proposed in 
 http://git.linuxtv.org/cgit.cgi/pinchartl/media.git/commit/?h=xilinx-wipid=fc094c354338861673464ed4b8fa198089fe7bf0.
 
 Hans, could you comment on that ?

Well, first of all your proposed API isn't merged yet, or even posted on the
mailinglist, so it's a bit unfair to require someone else to use it :-)

Also, while I do agree with your proposed new API I am a lot less enthusiastic
about creating yet another duplicate pad op for an existing audio/video routing
op.

The problem is that existing drivers are never updated for the new op and you 
are
stuck with competing internal APIs. Not nice at all.

Bottom line is that this driver uses s_routing like any other driver currently
in the kernel, so I have no problem with that.

Regards,

Hans

 
 +}

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v4] media: spi: Add support for LMH0395

2014-11-03 Thread Laurent Pinchart
On Monday 03 November 2014 14:36:26 Hans Verkuil wrote:
 On 11/03/2014 02:13 PM, Laurent Pinchart wrote:
  Hi Jean-Michel,
  
  Thank you for the patch.
 
  On Wednesday 10 September 2014 11:43:54 Jean-Michel Hautbois wrote:
 snip
 
  +static int lmh0395_s_routing(struct v4l2_subdev *sd, u32 input, u32
  output, +  u32 config)
  +{
  +  struct lmh0395_state *state = to_state(sd);
  +  int err = 0;
  +
  +  if (state-output_type != output)
  +  err = lmh0395_set_output_type(sd, output);
  +
  +  return err;
  
  if (state-output_type == output)
  return 0;
  
  return lmh0395_set_output_type(sd, output);
  
  You can then get rid of the err variable.
  
  I don't really like this implementation though, the output argument is
  device- specific, you thus require explicit knowledge of the LMH0395 in
  your bridge driver.
 
 Well, that's the way s_routing is defined. It's the bridge driver's job to
 translate between V4L2 inputs/outputs and low-level routing information.
 
  I'm not sure what the config argument is used for, but naively I would
  have
 
 config is normally not used. There are one or two drivers that need it for
 additional routing configuration, but it's rare.

OK.

  used it to tell whether to enable (1) or disable (0) the route from input
  to output. The input value should then always be 0, and the output value
  can be 1 or 2. Another option would be to use the new S_ROUTING userspace
  ioctl I've proposed in
  http://git.linuxtv.org/cgit.cgi/pinchartl/media.git/commit/?h=xilinx-wipi
  d=fc094c354338861673464ed4b8fa198089fe7bf0.
  
  Hans, could you comment on that ?
 
 Well, first of all your proposed API isn't merged yet, or even posted on the
 mailinglist, so it's a bit unfair to require someone else to use it :-)

It's not a requirement, just a proposal :-) I can send a patch series if 
there's enough interest for using that API.

 Also, while I do agree with your proposed new API I am a lot less
 enthusiastic about creating yet another duplicate pad op for an existing
 audio/video routing op.
 
 The problem is that existing drivers are never updated for the new op and
 you are stuck with competing internal APIs. Not nice at all.

Agreed, it's not nice, feel free to propose a better solution :-)

 Bottom line is that this driver uses s_routing like any other driver
 currently in the kernel, so I have no problem with that.

I do :-)

The bridge to subdev dependency is fine for PCI or USB devices where the 
hardware topology is known to the driver. It isn't for composite embedded 
devices, as we don't want to teach all bridges about all subdevs.

-- 
Regards,

Laurent Pinchart

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v4] media: spi: Add support for LMH0395

2014-11-03 Thread Hans Verkuil
On 11/03/2014 02:42 PM, Laurent Pinchart wrote:
 On Monday 03 November 2014 14:36:26 Hans Verkuil wrote:
 On 11/03/2014 02:13 PM, Laurent Pinchart wrote:
 Hi Jean-Michel,

 Thank you for the patch.

 On Wednesday 10 September 2014 11:43:54 Jean-Michel Hautbois wrote:
 snip

 +static int lmh0395_s_routing(struct v4l2_subdev *sd, u32 input, u32
 output, +  u32 config)
 +{
 +  struct lmh0395_state *state = to_state(sd);
 +  int err = 0;
 +
 +  if (state-output_type != output)
 +  err = lmh0395_set_output_type(sd, output);
 +
 +  return err;

 if (state-output_type == output)
 return 0;
 
 return lmh0395_set_output_type(sd, output);

 You can then get rid of the err variable.

 I don't really like this implementation though, the output argument is
 device- specific, you thus require explicit knowledge of the LMH0395 in
 your bridge driver.

 Well, that's the way s_routing is defined. It's the bridge driver's job to
 translate between V4L2 inputs/outputs and low-level routing information.

 I'm not sure what the config argument is used for, but naively I would
 have

 config is normally not used. There are one or two drivers that need it for
 additional routing configuration, but it's rare.
 
 OK.
 
 used it to tell whether to enable (1) or disable (0) the route from input
 to output. The input value should then always be 0, and the output value
 can be 1 or 2. Another option would be to use the new S_ROUTING userspace
 ioctl I've proposed in
 http://git.linuxtv.org/cgit.cgi/pinchartl/media.git/commit/?h=xilinx-wipi
 d=fc094c354338861673464ed4b8fa198089fe7bf0.

 Hans, could you comment on that ?

 Well, first of all your proposed API isn't merged yet, or even posted on the
 mailinglist, so it's a bit unfair to require someone else to use it :-)
 
 It's not a requirement, just a proposal :-) I can send a patch series if 
 there's enough interest for using that API.
 
 Also, while I do agree with your proposed new API I am a lot less
 enthusiastic about creating yet another duplicate pad op for an existing
 audio/video routing op.

 The problem is that existing drivers are never updated for the new op and
 you are stuck with competing internal APIs. Not nice at all.
 
 Agreed, it's not nice, feel free to propose a better solution :-)
 
 Bottom line is that this driver uses s_routing like any other driver
 currently in the kernel, so I have no problem with that.
 
 I do :-)
 
 The bridge to subdev dependency is fine for PCI or USB devices where the 
 hardware topology is known to the driver. It isn't for composite embedded 
 devices, as we don't want to teach all bridges about all subdevs.
 

I agree, and your proposal is a nice solution. But whoever want to get this
merged *will* have to take a good look at the existing g/s_routing ops and
how they can be converted to the new one. I didn't block that when similar
duplicate ops were added in the past and I am increasingly sorry I didn't do
that. It's creating incompatible subdevs, where the whole point of the subdev
API was to avoid that.

Regards,

Hans
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH v4] media: spi: Add support for LMH0395

2014-09-10 Thread Jean-Michel Hautbois
This device is a SPI based device from TI.
It is a 3 Gbps HD/SD SDI Dual Output Low Power
Extended Reach Adaptive Cable Equalizer.

LMH0395 enables the use of up to two outputs.
These can be configured using DT.

Controls should be accessible from userspace too.
This will have to be done later.

v2: Add DT support
v3: Change the bit set/clear in output_type as disabled means 'set the bit'
v4: Clearer description of endpoints usage in Doc, and some init changes.
Add a dependency on OF and don't test CONFIG_OF anymore.

Signed-off-by: Jean-Michel Hautbois 
---
 .../devicetree/bindings/media/spi/lmh0395.txt  |  48 +++
 MAINTAINERS|   6 +
 drivers/media/spi/Kconfig  |  14 +
 drivers/media/spi/Makefile |   1 +
 drivers/media/spi/lmh0395.c| 354 +
 5 files changed, 423 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/spi/lmh0395.txt
 create mode 100644 drivers/media/spi/Kconfig
 create mode 100644 drivers/media/spi/Makefile
 create mode 100644 drivers/media/spi/lmh0395.c

diff --git a/Documentation/devicetree/bindings/media/spi/lmh0395.txt 
b/Documentation/devicetree/bindings/media/spi/lmh0395.txt
new file mode 100644
index 000..7cc0026
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/spi/lmh0395.txt
@@ -0,0 +1,48 @@
+* Texas Instruments lmh0395 3G HD/SD SDI equalizer
+
+The LMH0395 3 Gbps HD/SD SDI Dual Output Low Power Extended Reach Adaptive
+Cable Equalizer is designed to equalize data transmitted over cable (or any
+media with similar dispersive loss characteristics).
+The equalizer operates over a wide range of data rates from 125 Mbps to 2.97 
Gbps
+and supports SMPTE 424M, SMPTE 292M, SMPTE344M, SMPTE 259M, and DVB-ASI 
standards.
+
+Required Properties :
+- compatible: Must be "ti,lmh0395"
+
+The device node must contain one 'port' child node per device input and output
+port, in accordance with the video interface bindings defined in
+Documentation/devicetree/bindings/media/video-interfaces.txt. The port nodes
+are numbered as follows.
+
+  Port LMH0395
+
+  SDI input0
+  SDI output   1,2
+
+Example:
+
+ecspi@0201 {
+   ...
+   ...
+
+   lmh0395@1 {
+   compatible = "ti,lmh0395";
+   reg = <1>;
+   spi-max-frequency = <2000>;
+   ports {
+   port@0 {
+   reg = <0>;
+   sdi0_in: endpoint {};
+   };
+   port@1 {
+   reg = <1>;
+   sdi0_out0: endpoint {};
+   };
+   port@2 {
+   reg = <2>;
+   /* endpoint not specified, disable output */
+   };
+   };
+   };
+   ...
+};
diff --git a/MAINTAINERS b/MAINTAINERS
index cf24bb5..ca42b9e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9141,6 +9141,12 @@ S:   Maintained
 F: sound/soc/codecs/lm49453*
 F: sound/soc/codecs/isabelle*
 
+TI LMH0395 DRIVER
+M: Jean-Michel Hautbois 
+L: linux-me...@vger.kernel.org
+S: Maintained
+F: drivers/media/spi/lmh0395*
+
 TI LP855x BACKLIGHT DRIVER
 M: Milo Kim 
 S: Maintained
diff --git a/drivers/media/spi/Kconfig b/drivers/media/spi/Kconfig
new file mode 100644
index 000..bcb1ab1
--- /dev/null
+++ b/drivers/media/spi/Kconfig
@@ -0,0 +1,14 @@
+if VIDEO_V4L2
+
+config VIDEO_LMH0395
+   tristate "LMH0395 equalizer"
+   depends on VIDEO_V4L2 && SPI && MEDIA_CONTROLLER && OF
+   ---help---
+ Support for TI LMH0395 3G HD/SD SDI Dual Output Low Power
+ Extended Reach Adaptive Cable Equalizer.
+
+ To compile this driver as a module, choose M here: the
+ module will be called lmh0395.
+
+
+endif
diff --git a/drivers/media/spi/Makefile b/drivers/media/spi/Makefile
new file mode 100644
index 000..6c587e5
--- /dev/null
+++ b/drivers/media/spi/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_VIDEO_LMH0395)+= lmh0395.o
diff --git a/drivers/media/spi/lmh0395.c b/drivers/media/spi/lmh0395.c
new file mode 100644
index 000..3eca0df
--- /dev/null
+++ b/drivers/media/spi/lmh0395.c
@@ -0,0 +1,354 @@
+/*
+ * LMH0395 SPI driver.
+ * Copyright (C) 2014  Jean-Michel Hautbois
+ *
+ * 3G HD/SD SDI Dual Output Low Power Extended Reach Adaptive Cable Equalizer
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the 

[PATCH v4] media: spi: Add support for LMH0395

2014-09-10 Thread Jean-Michel Hautbois
This device is a SPI based device from TI.
It is a 3 Gbps HD/SD SDI Dual Output Low Power
Extended Reach Adaptive Cable Equalizer.

LMH0395 enables the use of up to two outputs.
These can be configured using DT.

Controls should be accessible from userspace too.
This will have to be done later.

v2: Add DT support
v3: Change the bit set/clear in output_type as disabled means 'set the bit'
v4: Clearer description of endpoints usage in Doc, and some init changes.
Add a dependency on OF and don't test CONFIG_OF anymore.

Signed-off-by: Jean-Michel Hautbois jean-michel.hautb...@vodalys.com
---
 .../devicetree/bindings/media/spi/lmh0395.txt  |  48 +++
 MAINTAINERS|   6 +
 drivers/media/spi/Kconfig  |  14 +
 drivers/media/spi/Makefile |   1 +
 drivers/media/spi/lmh0395.c| 354 +
 5 files changed, 423 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/spi/lmh0395.txt
 create mode 100644 drivers/media/spi/Kconfig
 create mode 100644 drivers/media/spi/Makefile
 create mode 100644 drivers/media/spi/lmh0395.c

diff --git a/Documentation/devicetree/bindings/media/spi/lmh0395.txt 
b/Documentation/devicetree/bindings/media/spi/lmh0395.txt
new file mode 100644
index 000..7cc0026
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/spi/lmh0395.txt
@@ -0,0 +1,48 @@
+* Texas Instruments lmh0395 3G HD/SD SDI equalizer
+
+The LMH0395 3 Gbps HD/SD SDI Dual Output Low Power Extended Reach Adaptive
+Cable Equalizer is designed to equalize data transmitted over cable (or any
+media with similar dispersive loss characteristics).
+The equalizer operates over a wide range of data rates from 125 Mbps to 2.97 
Gbps
+and supports SMPTE 424M, SMPTE 292M, SMPTE344M, SMPTE 259M, and DVB-ASI 
standards.
+
+Required Properties :
+- compatible: Must be ti,lmh0395
+
+The device node must contain one 'port' child node per device input and output
+port, in accordance with the video interface bindings defined in
+Documentation/devicetree/bindings/media/video-interfaces.txt. The port nodes
+are numbered as follows.
+
+  Port LMH0395
+
+  SDI input0
+  SDI output   1,2
+
+Example:
+
+ecspi@0201 {
+   ...
+   ...
+
+   lmh0395@1 {
+   compatible = ti,lmh0395;
+   reg = 1;
+   spi-max-frequency = 2000;
+   ports {
+   port@0 {
+   reg = 0;
+   sdi0_in: endpoint {};
+   };
+   port@1 {
+   reg = 1;
+   sdi0_out0: endpoint {};
+   };
+   port@2 {
+   reg = 2;
+   /* endpoint not specified, disable output */
+   };
+   };
+   };
+   ...
+};
diff --git a/MAINTAINERS b/MAINTAINERS
index cf24bb5..ca42b9e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9141,6 +9141,12 @@ S:   Maintained
 F: sound/soc/codecs/lm49453*
 F: sound/soc/codecs/isabelle*
 
+TI LMH0395 DRIVER
+M: Jean-Michel Hautbois jean-michel.hautb...@vodalys.com
+L: linux-me...@vger.kernel.org
+S: Maintained
+F: drivers/media/spi/lmh0395*
+
 TI LP855x BACKLIGHT DRIVER
 M: Milo Kim milo@ti.com
 S: Maintained
diff --git a/drivers/media/spi/Kconfig b/drivers/media/spi/Kconfig
new file mode 100644
index 000..bcb1ab1
--- /dev/null
+++ b/drivers/media/spi/Kconfig
@@ -0,0 +1,14 @@
+if VIDEO_V4L2
+
+config VIDEO_LMH0395
+   tristate LMH0395 equalizer
+   depends on VIDEO_V4L2  SPI  MEDIA_CONTROLLER  OF
+   ---help---
+ Support for TI LMH0395 3G HD/SD SDI Dual Output Low Power
+ Extended Reach Adaptive Cable Equalizer.
+
+ To compile this driver as a module, choose M here: the
+ module will be called lmh0395.
+
+
+endif
diff --git a/drivers/media/spi/Makefile b/drivers/media/spi/Makefile
new file mode 100644
index 000..6c587e5
--- /dev/null
+++ b/drivers/media/spi/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_VIDEO_LMH0395)+= lmh0395.o
diff --git a/drivers/media/spi/lmh0395.c b/drivers/media/spi/lmh0395.c
new file mode 100644
index 000..3eca0df
--- /dev/null
+++ b/drivers/media/spi/lmh0395.c
@@ -0,0 +1,354 @@
+/*
+ * LMH0395 SPI driver.
+ * Copyright (C) 2014  Jean-Michel Hautbois
+ *
+ * 3G HD/SD SDI Dual Output Low Power Extended Reach Adaptive Cable Equalizer
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be