Re: [PATCH v7 2/3] media: i2c: adv748x: add adv748x driver

2017-07-18 Thread Sakari Ailus
Hi Kieran,

A few more minor matters that you might want to address on top of Hans's
pull request.

On Thu, Jul 06, 2017 at 12:01:16PM +0100, Kieran Bingham wrote:
...
> +static int adv748x_afe_g_input_status(struct v4l2_subdev *sd, u32 *status)
> +{
> + struct adv748x_afe *afe = adv748x_sd_to_afe(sd);
> + struct adv748x_state *state = adv748x_afe_to_state(afe);
> + int ret;
> +
> + mutex_lock(&state->mutex);
> +
> + ret = adv748x_afe_status(afe, status, NULL);
> +
> + mutex_unlock(&state->mutex);

A newline here would be nice.

> + return ret;
> +}

...

> +int adv748x_csi2_set_pixelrate(struct v4l2_subdev *sd, s64 rate)
> +{
> + struct v4l2_ctrl *ctrl;
> +
> + ctrl = v4l2_ctrl_find(sd->ctrl_handler, V4L2_CID_PIXEL_RATE);

It'd be much nicer to store the control pointer to your device's own struct
and use it. No need to look it up or check whether it was found.

> + if (!ctrl)
> + return -EINVAL;
> +
> + return v4l2_ctrl_s_ctrl_int64(ctrl, rate);
> +}

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk


[PATCH v7 2/3] media: i2c: adv748x: add adv748x driver

2017-07-06 Thread Kieran Bingham
From: Kieran Bingham 

Provide support for the ADV7481 and ADV7482.

The driver is modelled with 4 subdevices to allow simultaneous streaming
from the AFE (Analog front end) and HDMI inputs though two CSI TX
entities.

The HDMI entity is linked to the TXA CSI bus, whilst the AFE is linked
to the TXB CSI bus.

The driver is based on a prototype by Koji Matsuoka in the Renesas BSP,
and an earlier rework by Niklas Söderlund.

Signed-off-by: Kieran Bingham 

---

v2:
 - Implement DT parsing
 - adv748x: Add CSI2 entity
 - adv748x: Rework pad allocations and fmts
 - Give AFE 8 input pads and move pad defines
 - Use the enums to ensure pads are referenced correctly.
 - adv748x: Rename AFE/HDMI entities
   Now they are 'just afe' and 'just hdmi'
 - Reorder the entity enum and structures
 - Added pad-format for the CSI2 entities
 - CSI2 s_stream pass through
 - CSI2 control pass through (with link following)

v3:
 - dt: Extend DT documentation to specify interrupt mappings
 - simplify adv748x_parse_dt
 - core: Add banner to header file describing ADV748x variants
 - Use entity structure pointers rather than global state pointers where
   possible
 - afe: Reduce indent on afe_status
 - hdmi: Add error checking to the bt->pixelclock values.
 - Remove all unnecessary pad checks: handled by core
 - Fix all probe cleanup paths
 - adv748x_csi2_probe() now fails if it has no endpoint
 - csi2: Fix small oneliners for is_txa and get_remote_sd()
 - csi2: Fix checkpatch warnings
 - csi2: Fix up s_stream pass-through
 - csi2: Fix up Pixel Rate passthrough
 - csi2: simplify adv748x_csi2_get_pad_format()
 - Remove 'async notifiers' from AFE/HDMI
   Using async notifiers was overkill, when we have access to the
   subdevices internally and can register them directly.
 - Use state lock in control handlers and clean up s_ctrls
 - remove _interruptible mutex locks

v4:
 - all: Convert hex 0xXX to lowercase
 - all: Use defines instead of hardcoded register values
 - all: Use regmap
 - afe, csi2, hdmi: _probe -> _init
 - afe, csi2, hdmi: _remove -> _cleanup
 - afe, hdmi: Convert pattern generator to a control
 - afe, hdmi: get/set-fmt refactor
 - afe, hdmi, csi2: Convert to internal calls for pixelrate
 - afe: Allow the AFE to configure the Input Select from DT
 - afe: Reduce indent on adv748x_afe_status switch
 - afe: Remove ununsed macro definitions AIN0-7
 - afe: Remove extraneous control checks handled by core
 - afe: Comment fixups
 - afe: Rename error label
 - afe: Correct control names on the SDP
 - afe: Use AIN0-7 rather than AIN1-8 to match ports and MC pads
 - core: adv748x_parse_dt references to nodes, and catch multiple
   endpoints in a port.
 - core: adv748x_dt_cleanup to simplify releasing DT nodes
 - core: adv748x_print_info renamed to adv748x_identify_chip
 - core: reorganise ordering of probe sequence
 - core: No need for of_match_ptr
 - core: Fix up i2c read/writes (regmap still on todo list)
 - core: Set specific functions from the entities on subdev-init
 - core: Use kzalloc for state instead of devm
 - core: Improve probe error reporting
 - core: Track unknown BIT(6) in tx{a,b}_power
 - csi2: Improve adv748x_csi2_get_remote_sd as adv748x_csi2_get_source_sd
 - csi2: -EPIPE instead of -ENODEV
 - csi2: adv_dbg, instead of adv_info
 - csi2: adv748x_csi2_set_format fix
 - csi2: remove async notifier and sd member variables
 - csi2: register links from the CSI2
 - csi2: create virtual channel helper function
 - dt: Remove numbering from endpoints
 - dt: describe ports and interrupts as optional
 - dt: Re-tab
 - hdmi: adv748x_hdmi_have_signal -> adv748x_hdmi_has_signal
 - hdmi: fix adv748x_hdmi_read_pixelclock return checks
 - hdmi: improve adv748x_hdmi_set_video_timings
 - hdmi: Fix tmp variable as polarity
 - hdmi: Improve adv748x_hdmi_s_stream
 - hdmi: Clean up adv748x_hdmi_s_ctrl register definitions and usage
 - hdmi: Fix up adv748x_hdmi_s_dv_timings with macro names for register
 - hdmi: Add locking to adv748x_hdmi_g_dv_timings
   writes and locking
 - hdmi: adv748x_hdmi_set_de_timings function added to clarify DE writes
 - hdmi: Use CP in control register naming to match component processor
 - hdmi: clean up adv748x_hdmi_query_dv_timings()
 - KConfig: Fix up dependency and capitalisation

v5:
 - afe,hdmi: _set_pixelrate -> _propagate_pixelrate
 - hdmi: Fix arm32 compilation failure : Use DIV_ROUND_CLOSEST_ULL
 - core: remove unused link functions
 - csi2: Use immutable links for HDMI->TXA, AFE->TXB

v6:
 - hdmi: Provide EDID support
 - afe: Fix InLock inversion bug
 - afe: Prevent autodetection of input format except in querystd
 - afe,hdmi: Improve pattern generator control strings
 - hdmi: Remove interlaced support capability (it's untested)

v7:
 - afe: Initialise std to V4L2_STD_NTSC_M and restore after query
 - afe: use V4L2_FIELD_ALTERNATE
 - hdmi: Define a minimum pixelclock value
 - hdmi: Remove interlaced formats as they are untested
 - Kconfig: Select REGMAP_I2C
 - core: include linux/slab.h for kzalloc