Hi Jean-Michel,
Thank you for the patch.
On Friday 01 August 2014 16:04:37 jean-michel.hautbois@xxxxxxxxxxx wrote:
> From: Jean-Michel Hautbois <jean-michel.hautbois@xxxxxxxxxxx>
>
> 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.
>
> Add routing support in order to select output
> LMH0395 enables the use of up to two outputs.
>
> Signed-off-by: Jean-Michel Hautbois <jean-michel.hautbois@xxxxxxxxxxx>
> ---
> .../devicetree/bindings/media/spi/lmh0395.txt | 24 ++
> drivers/media/Kconfig | 1 +
> drivers/media/Makefile | 2 +-
> drivers/media/spi/Kconfig | 14 ++
> drivers/media/spi/Makefile | 1 +
> drivers/media/spi/lmh0395.c | 256 ++++++++++++++++++
> 6 files changed, 297 insertions(+), 1 deletion(-)
> 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 0000000..d55c4eb
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/spi/lmh0395.txt
> @@ -0,0 +1,24 @@
> +* 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"
As the device has ports, they should be exposed in DT using the of-graph
bindings. You can just reference the video-interfaces.txt document. See the
adv7604 bindings for an example.
> +
> +Example:
> +
> +ecspi@02010000 {
> + ...
> + ...
> +
> + lmh0395@1 {
> + compatible = "ti,lmh0395";
> + reg = <1>;
> + spi-max-frequency = <20000000>;
> + };
> + ...
> +};
> diff --git a/drivers/media/Kconfig b/drivers/media/Kconfig
> index 1d0758a..ce193b0 100644
> --- a/drivers/media/Kconfig
> +++ b/drivers/media/Kconfig
> @@ -199,6 +199,7 @@ config MEDIA_ATTACH
> default MODULES
>
> source "drivers/media/i2c/Kconfig"
> +source "drivers/media/spi/Kconfig"
> source "drivers/media/tuners/Kconfig"
> source "drivers/media/dvb-frontends/Kconfig"
>
> diff --git a/drivers/media/Makefile b/drivers/media/Makefile
> index 620f275..7578bcd 100644
> --- a/drivers/media/Makefile
> +++ b/drivers/media/Makefile
> @@ -28,6 +28,6 @@ obj-y += rc/
> # Finally, merge the drivers that require the core
> #
>
> -obj-y += common/ platform/ pci/ usb/ mmc/ firewire/ parport/
> +obj-y += common/ platform/ pci/ usb/ mmc/ firewire/ parport/ spi/
> obj-$(CONFIG_VIDEO_DEV) += radio/
>
> diff --git a/drivers/media/spi/Kconfig b/drivers/media/spi/Kconfig
> new file mode 100644
> index 0000000..291e7ea
> --- /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
> + ---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 0000000..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 0000000..e287725
> --- /dev/null
> +++ b/drivers/media/spi/lmh0395.c
> @@ -0,0 +1,256 @@
> +/*
> + * 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 implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/ioctl.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/types.h>
> +#include <linux/slab.h>
> +#include <linux/uaccess.h>
> +#include <linux/spi/spi.h>
> +#include <linux/videodev2.h>
> +#include <media/v4l2-device.h>
> +
> +static int debug;
> +module_param(debug, int, 0644);
> +MODULE_PARM_DESC(debug, "debug level (0-1)");
> +
> +#define LMH0395_SPI_CMD_WRITE 0x00
> +#define LMH0395_SPI_CMD_READ 0x80
> +
> +/* Registers of LMH0395 */
> +#define LMH0395_GENERAL_CTRL 0x00
> +#define LMH0395_OUTPUT_DRIVER 0x01
> +#define LMH0395_LAUNCH_AMP_CTRL 0x02
> +#define LMH0395_MUTE_REF 0x03
> +#define LMH0395_DEVICE_ID 0x04
> +#define LMH0395_RATE_INDICATOR 0x05
> +#define LMH0395_CABLE_LENGTH_INDICATOR 0x06
> +#define LMH0395_LAUNCH_AMP_INDICATION 0x07
> +
> +/* This is a one input, dual output device */
> +#define LMH0395_SDI_INPUT 0
> +#define LMH0395_SDI_OUT0 1
> +#define LMH0395_SDI_OUT1 2
> +
> +#define LMH0395_PADS_NUM 3
> +
> +/* Register LMH0395_MUTE_REF bits [7:6] */
> +enum lmh0395_output_type {
> + LMH0395_OUTPUT_TYPE_NONE,
> + LMH0395_OUTPUT_TYPE_SDO0,
> + LMH0395_OUTPUT_TYPE_SDO1,
> + LMH0395_OUTPUT_TYPE_BOTH
> +};
> +
> +static const char * const output_strs[] = {
> + "No Output Driver",
> + "Output Driver 0",
> + "Output Driver 1",
> + "Output Driver 0+1",
> +};
> +
> +
> +/* spi implementation */
> +
> +static int lmh0395_spi_write(struct spi_device *spi, u8 reg, u8 data)
> +{
> + int err = 0;
No need to initialize err to 0 here.
> + u8 cmd[2];
> +
> + cmd[0] = LMH0395_SPI_CMD_WRITE | reg;
> + cmd[1] = data;
> +
> + err = spi_write(spi, cmd, 2);
> + if (err < 0) {
> + dev_err(&spi->dev, "SPI failed to select reg\n");
> + return err;
> + }
> +
> + return err;
> +}
> +
> +static int lmh0395_spi_read(struct spi_device *spi, u8 reg, u8 *data)
> +{
> + int err = 0;
Same comment.
> + u8 cmd[2];
> + u8 read_data[2];
> +
> + cmd[0] = LMH0395_SPI_CMD_READ | reg;
> + cmd[1] = 0xFF;
> +
> + err = spi_write(spi, cmd, 2);
> + if (err < 0) {
> + dev_err(&spi->dev, "SPI failed to select reg\n");
> + return err;
> + }
> +
> + err = spi_read(spi, read_data, 2);
> + if (err < 0) {
> + dev_err(&spi->dev, "SPI failed to read reg\n");
> + return err;
> + }
> + /* The first 8 bits is the adress used, drop it */
> + *data = read_data[1];
> +
> + return err;
> +}
> +
> +struct lmh0395_state {
> + struct v4l2_subdev sd;
> + struct media_pad pads[LMH0395_PADS_NUM];
> + enum lmh0395_output_type output_type;
> +};
> +
> +static inline struct lmh0395_state *to_state(struct v4l2_subdev *sd)
> +{
> + return container_of(sd, struct lmh0395_state, sd);
> +}
> +
> +static int lmh0395_set_output_type(struct v4l2_subdev *sd, u32 output)
> +{
> + struct lmh0395_state *state = to_state(sd);
> + struct spi_device *spi = v4l2_get_subdevdata(sd);
> + u8 muteref_reg;
> +
> + switch (output) {
> + case LMH0395_OUTPUT_TYPE_SDO0:
> + lmh0395_spi_read(spi, LMH0395_MUTE_REF, &muteref_reg);
> + muteref_reg |= 0x01 << 6;
> + break;
> + case LMH0395_OUTPUT_TYPE_SDO1:
> + lmh0395_spi_read(spi, LMH0395_MUTE_REF, &muteref_reg);
> + muteref_reg |= 0x10 << 6;
> + break;
> + case LMH0395_OUTPUT_TYPE_BOTH:
> + lmh0395_spi_read(spi, LMH0395_MUTE_REF, &muteref_reg);
> + muteref_reg |= 0x11 << 6;
> + break;
> + case LMH0395_OUTPUT_TYPE_NONE:
> + lmh0395_spi_read(spi, LMH0395_MUTE_REF, &muteref_reg);
> + muteref_reg |= 0x0 << 6;
> + break;
> + default:
> + return -EINVAL;
> + }
> + v4l2_info(sd, "Selecting %s output type\n", output_strs[output]);
v4l2_info will print a message each time the output is changed, that will spam
the kernel log. You can turn that into a debug message.
> +
> + lmh0395_spi_write(spi, LMH0395_MUTE_REF, muteref_reg);
> + state->output_type = output;
> + return 0;
> +
> +}
> +
> +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;
> +}
> +
> +static const struct v4l2_subdev_video_ops lmh0395_video_ops = {
> + .s_routing = lmh0395_s_routing,
> +};
> +
> +static const struct v4l2_subdev_ops lmh0395_ops = {
> + .video = &lmh0395_video_ops,
> +};
> +
> +static int lmh0395_probe(struct spi_device *spi)
> +{
> + u8 device_id;
> + struct lmh0395_state *state;
> + struct v4l2_subdev *sd;
> + int err;
> +
> + err = lmh0395_spi_read(spi, LMH0395_DEVICE_ID, &device_id);
> + if (err < 0)
> + return err;
> +
> + dev_dbg(&spi->dev, "device_id 0x%x\n", device_id);
> +
> + /* Now that the device is here, let's init V4L2 */
> + state = devm_kzalloc(&spi->dev, sizeof(*state), GFP_KERNEL);
> + if (!state)
> + return -ENOMEM;
> +
> + sd = &state->sd;
> + v4l2_spi_subdev_init(sd, spi, &lmh0395_ops);
> + state->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
The only subdev operation implemented by the driver, video.s_routing, isn't
exposed to userspace by the subdev APÏ, so the node would be pretty useless.
I've started working on a more generic routing operation that could be exposed
to userspace. I'll try to post patches in the near future.
> +
> + state->pads[LMH0395_SDI_INPUT].flags = MEDIA_PAD_FL_SINK;
> + state->pads[LMH0395_SDI_OUT0].flags = MEDIA_PAD_FL_SOURCE;
> + state->pads[LMH0395_SDI_OUT1].flags = MEDIA_PAD_FL_SOURCE;
> + err = media_entity_init(&sd->entity, LMH0395_PADS_NUM, &state->pads, 0);
> + if (err)
> + return err;
> +
> + err = v4l2_async_register_subdev(sd);
> + if (err < 0)
> + media_entity_cleanup(&sd->entity);
Shouldn't you return err here ?
> + v4l2_dbg(1, debug, sd, "Configuring equalizer\n");
> + lmh0395_set_output_type(sd, LMH0395_OUTPUT_TYPE_BOTH);
> + dev_info(&spi->dev, "driver registered\n");
That's incorrect, the probe function doesn't register the driver. If you want
a message here, you could say "device probed" or "device found". The other
option would be to just remove the message, or turn it into a dev_dbg.
> +
> + return 0;
> +}
> +
> +static int lmh0395_remove(struct spi_device *spi)
> +{
> + struct v4l2_subdev *sd = spi_get_drvdata(spi);
> +
> + v4l2_async_unregister_subdev(sd);
> + media_entity_cleanup(&sd->entity);
> + return 0;
> +}
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
