Synopsys Designware HDMI Video Capture Controller + PHY
Subsystems (2)
v6 cover Synopsys Designware HDMI Video Capture Controller + PHY
The Synopsys Designware HDMI RX controller is an HDMI receiver controller that is responsible to process digital data that comes from a phy. The final result is a stream of raw video data that can then be connected to a video DMA, for example, and transfered into RAM so that it can be displayed.
The controller + phy available in this series natively support all HDMI 1.4 and HDMI 2.0 modes, including deep color. Although, the driver is quite in its initial stage and unfortunatelly only non deep color modes are supported. Also, audio is not yet supported in the driver (the controller has several audio output interfaces).
Version 6 addresses review comments from Hans Verkuil regarding CEC, HDCP 1.4 and adds the s_dv_timings() callback, as well as the V4L2_CID_DV_RX_POWER_PRESENT ctrl.
This series depends on the patch at [1].
This series was tested in a FPGA platform using an embedded platform called ARC AXS101.
Jose Abreu (4):
[media] platform: Add Synopsys Designware HDMI RX PHY e405 Driver [media] platform: Add Synopsys Designware HDMI RX Controller Driver
MAINTAINERS: Add entry for Synopsys Designware HDMI drivers
dt-bindings: media: Document Synopsys Designware HDMI RX
[1] https://patchwork.linuxtv.org/patch/41834/
.../devicetree/bindings/media/snps,dw-hdmi-rx.txt | 70 + MAINTAINERS | 7 + drivers/media/platform/Kconfig | 2 + drivers/media/platform/Makefile | 2 + drivers/media/platform/dwc/Kconfig | 23 + drivers/media/platform/dwc/Makefile | 2 + drivers/media/platform/dwc/dw-hdmi-phy-e405.c | 844 +++++++++ drivers/media/platform/dwc/dw-hdmi-phy-e405.h | 63 + drivers/media/platform/dwc/dw-hdmi-rx.c | 1809 ++++++++++++++++++++ drivers/media/platform/dwc/dw-hdmi-rx.h | 441 +++++ include/media/dwc/dw-hdmi-phy-pdata.h | 128 ++ include/media/dwc/dw-hdmi-rx-pdata.h | 70 + 12 files changed, 3461 insertions(+) create mode 100644 Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt create mode 100644 drivers/media/platform/dwc/Kconfig create mode 100644 drivers/media/platform/dwc/Makefile create mode 100644 drivers/media/platform/dwc/dw-hdmi-phy-e405.c create mode 100644 drivers/media/platform/dwc/dw-hdmi-phy-e405.h create mode 100644 drivers/media/platform/dwc/dw-hdmi-rx.c create mode 100644 drivers/media/platform/dwc/dw-hdmi-rx.h create mode 100644 include/media/dwc/dw-hdmi-phy-pdata.h create mode 100644 include/media/dwc/dw-hdmi-rx-pdata.h
Signature · 1 line
1.9.1
PATCH v6 1/4 platform: Add Synopsys Designware HDMI RX PHY e405 Driver
This adds support for the Synopsys Designware HDMI RX PHY e405. This phy receives and decodes HDMI video that is delivered to a controller.
Main features included in this driver are:
- Equalizer algorithm that chooses the phy best settings according to the detected HDMI cable characteristics.
- Support for scrambling
- Support for HDMI 2.0 modes up to 6G (HDMI 4k@60Hz).
The driver was implemented as a standalone V4L2 subdevice and the phy interface with the controller was implemented using V4L2 ioctls. I do not know if this is the best option but it is not possible to use the existing API functions directly as we need specific functions that will be called by the controller at specific configuration stages.
There is also a bidirectional communication between controller and phy:
The phy must provide functions that the controller will call (i.e. configuration functions) and the controller must provide read/write callbacks, as well as other specific functions.
Changes from v4:
- Use usleep_range (Sylwester)
- Remove some comments (Sylwester)
- Parse phy version from of_device_id (Sylwester)
- Use "cfg" instead of "cfg-clk" (Sylwester, Rob)
- Change some messages to dev_dbg (Sylwester)
Changes from v3:
- Use v4l2 async API (Sylwester)
- Use clock API (Sylwester)
- Add compatible string (Sylwester)
Changes from RFC:
- Remove a bunch of functions that can be collapsed into a single config() function
- Add comments for the callbacks and structures (Hans)
--- drivers/media/platform/Kconfig | 2 + drivers/media/platform/Makefile | 2 + drivers/media/platform/dwc/Kconfig | 8 + drivers/media/platform/dwc/Makefile | 1 + drivers/media/platform/dwc/dw-hdmi-phy-e405.c | 844 ++++++++++++++++++++++++++ drivers/media/platform/dwc/dw-hdmi-phy-e405.h | 63 ++ include/media/dwc/dw-hdmi-phy-pdata.h | 128 ++++ 7 files changed, 1048 insertions(+) create mode 100644 drivers/media/platform/dwc/Kconfig create mode 100644 drivers/media/platform/dwc/Makefile create mode 100644 drivers/media/platform/dwc/dw-hdmi-phy-e405.c create mode 100644 drivers/media/platform/dwc/dw-hdmi-phy-e405.h create mode 100644 include/media/dwc/dw-hdmi-phy-pdata.h
drivers/media/platform/Kconfig +2 -0
@@ -33,6 +33,8 @@ source "drivers/media/platform/omap/Kconfig" source "drivers/media/platform/blackfin/Kconfig" +source "drivers/media/platform/dwc/Kconfig" + config VIDEO_SH_VOU tristate "SuperH VOU video output driver" depends on MEDIA_CAMERA_SUPPORT
drivers/media/platform/Makefile +2 -0
@@ -86,3 +86,5 @@ obj-$(CONFIG_VIDEO_MEDIATEK_MDP) += mtk-mdp/ obj-$(CONFIG_VIDEO_MEDIATEK_JPEG) += mtk-jpeg/ obj-$(CONFIG_VIDEO_QCOM_VENUS) += qcom/venus/ + +obj-y += dwc/
drivers/media/platform/dwc/Kconfig new file +8 -0
@@ -0,0 +1,8 @@ +config VIDEO_DWC_HDMI_PHY_E405 + tristate "Synopsys Designware HDMI RX PHY e405 driver" + depends on VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API + help + Support for Synopsys Designware HDMI RX PHY. Version is e405. + + To compile this driver as a module, choose M here. The module + will be called dw-hdmi-phy-e405.
drivers/media/platform/dwc/Makefile new file +1 -0
@@ -0,0 +1 @@ +obj-$(CONFIG_VIDEO_DWC_HDMI_PHY_E405) += dw-hdmi-phy-e405.o
drivers/media/platform/dwc/dw-hdmi-phy-e405.c new file +844 -0
844 lines not on this page · raw message
drivers/media/platform/dwc/dw-hdmi-phy-e405.h new file +63 -0
63 lines not on this page · raw message
include/media/dwc/dw-hdmi-phy-pdata.h new file +128 -0
128 lines not on this page · raw message
Signature · 1 line
1.9.1
PATCH v6 2/4 platform: Add Synopsys Designware HDMI RX Controller Driver
In reply to Jose AbreuThis is an initial submission for the Synopsys Designware HDMI RX Controller Driver. This driver interacts with a phy driver so that a communication between them is created and a video pipeline is configured.
The controller + phy pipeline can then be integrated into a fully featured system that can be able to receive video up to 4k@60Hz with deep color 48bit RGB, depending on the platform. Although, this initial version does not yet handle deep color modes.
This driver was implemented as a standard V4L2 subdevice and its main features are:
- Internal state machine that reconfigures phy until the video is not stable
- JTAG communication with phy
- Inter-module communication with phy driver
- Debug write/read ioctls
Some notes:
- RX sense controller (cable connection/disconnection) must be handled by the platform wrapper as this is not integrated into the controller RTL
- The same goes for EDID ROM's
- ZCAL calibration is needed only in FPGA platforms, in ASIC this is not needed
- The state machine is not an ideal solution as it creates a kthread but it is needed because some sources might not be very stable at sending the video (i.e. we must react accordingly).
Changes from v5:
- Removed HDCP 1.4 support (Hans)
- Removed some CEC debug messages (Hans)
- Add s_dv_timings callback (Hans)
- Add V4L2_CID_DV_RX_POWER_PRESENT ctrl (Hans)
Changes from v4:
- Add flag V4L2_SUBDEV_FL_HAS_DEVNODE (Sylwester)
- Remove some comments and change some messages to dev_dbg (Sylwester)
- Use v4l2_async_subnotifier_register() (Sylwester)
Changes from v3:
- Use v4l2 async API (Sylwester)
- Do not block waiting for phy
- Do not use busy waiting delays (Sylwester)
- Simplify dw_hdmi_power_on (Sylwester)
- Use clock API (Sylwester)
- Use compatible string (Sylwester)
- Minor fixes (Sylwester)
Changes from v2:
- Address review comments from Hans regarding CEC
- Use CEC notifier
- Enable SCDC
Changes from v1:
- Add support for CEC
- Correct typo errors
- Correctly detect interlaced video modes
- Correct VIC parsing
Changes from RFC:
- Add support for HDCP 1.4
- Fixup HDMI_VIC not being parsed (Hans)
- Send source change signal when powering off (Hans)
- Add a "wait stable delay"
- Detect interlaced video modes (Hans)
- Restrain g/s_register from reading/writing to HDCP regs (Hans)
--- drivers/media/platform/dwc/Kconfig | 15 + drivers/media/platform/dwc/Makefile | 1 + drivers/media/platform/dwc/dw-hdmi-rx.c | 1809 +++++++++++++++++++++++++++++++ drivers/media/platform/dwc/dw-hdmi-rx.h | 441 ++++++++ include/media/dwc/dw-hdmi-rx-pdata.h | 70 ++ 5 files changed, 2336 insertions(+) create mode 100644 drivers/media/platform/dwc/dw-hdmi-rx.c create mode 100644 drivers/media/platform/dwc/dw-hdmi-rx.h create mode 100644 include/media/dwc/dw-hdmi-rx-pdata.h
drivers/media/platform/dwc/Kconfig +15 -0
@@ -6,3 +6,18 @@ config VIDEO_DWC_HDMI_PHY_E405 To compile this driver as a module, choose M here. The module will be called dw-hdmi-phy-e405. + +config VIDEO_DWC_HDMI_RX + tristate "Synopsys Designware HDMI Receiver driver" + depends on VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API + help + Support for Synopsys Designware HDMI RX controller. + + To compile this driver as a module, choose M here. The module + will be called dw-hdmi-rx. + +config VIDEO_DWC_HDMI_RX_CEC + bool + depends on VIDEO_DWC_HDMI_RX + select CEC_CORE + select CEC_NOTIFIER
drivers/media/platform/dwc/Makefile +1 -0
@@ -1 +1,2 @@ obj-$(CONFIG_VIDEO_DWC_HDMI_PHY_E405) += dw-hdmi-phy-e405.o +obj-$(CONFIG_VIDEO_DWC_HDMI_RX) += dw-hdmi-rx.o
drivers/media/platform/dwc/dw-hdmi-rx.c new file +1809 -0
1,809 lines not on this page · raw message
drivers/media/platform/dwc/dw-hdmi-rx.h new file +441 -0
441 lines not on this page · raw message
include/media/dwc/dw-hdmi-rx-pdata.h new file +70 -0
70 lines not on this page · raw message
Signature · 1 line
1.9.1
kbuild test robot
build botRe: [PATCH v6 2/4] [media] platform: Add Synopsys Designware HDMI RX Controller Driver
reproduce: wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree make.cross…
Hi Jose,
[auto build test ERROR on linuxtv-media/master]
[also build test ERROR on next-20170706]
[cannot apply to v4.12]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Jose-Abreu/Synopsys-Designware-HDMI-Video-Capture-Controller-PHY/20170707-041312 base: git://linuxtv.org/media_tree.git master config: blackfin-allyesconfig (attached as .config) compiler: bfin-uclinux-gcc (GCC) 6.2.0
reproduce:
wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=blackfin
All errors (new ones prefixed by >>):
drivers/media//platform/dwc/dw-hdmi-rx.c: In function 'dw_hdmi_registered':
>> drivers/media//platform/dwc/dw-hdmi-rx.c:1452:9: error: implicit declaration of function 'v4l2_async_subnotifier_register' [-Werror=implicit-function-declaration]
return v4l2_async_subnotifier_register(&dw_dev->sd,
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/media//platform/dwc/dw-hdmi-rx.c: In function 'dw_hdmi_unregistered':
>> drivers/media//platform/dwc/dw-hdmi-rx.c:1462:2: error: implicit declaration of function 'v4l2_async_subnotifier_unregister' [-Werror=implicit-function-declaration]
v4l2_async_subnotifier_unregister(&dw_dev->v4l2_notifier);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/media//platform/dwc/dw-hdmi-rx.c: In function 'dw_hdmi_parse_dt':
drivers/media//platform/dwc/dw-hdmi-rx.c:1555:22: warning: unused variable 'notifier' [-Wunused-variable]
struct device_node *notifier, *phy_node, *np = dw_dev->of_node;
^~~~~~~~
drivers/media//platform/dwc/dw-hdmi-rx.c: In function 'dw_hdmi_rx_probe':
drivers/media//platform/dwc/dw-hdmi-rx.c:1765:1: warning: label 'err_phy' defined but not used [-Wunused-label]
err_phy:
^~~~~~~cc1: some warnings being treated as errors
vim +/v4l2_async_subnotifier_register +1452 drivers/media//platform/dwc/dw-hdmi-rx.c
1446 return ret;
1447 }
1448
1449 cec_register_cec_notifier(dw_dev->cec_adap, dw_dev->cec_notifier);
1450 dw_dev->registered = true;
1451
> 1452 return v4l2_async_subnotifier_register(&dw_dev->sd,
1453 &dw_dev->v4l2_notifier);
1454 }
1455
1456 static void dw_hdmi_unregistered(struct v4l2_subdev *sd)
1457 {
1458 struct dw_hdmi_dev *dw_dev = to_dw_dev(sd);
1459
1460 cec_unregister_adapter(dw_dev->cec_adap);
1461 cec_notifier_put(dw_dev->cec_notifier);
> 1462 v4l2_async_subnotifier_unregister(&dw_dev->v4l2_notifier); 1463 }
1464
1465 static const struct v4l2_subdev_core_ops dw_hdmi_sd_core_ops = {
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
Re: [PATCH v6 2/4] [media] platform: Add Synopsys Designware HDMI RX Controller Driver
Quoted 7 lines · kbuild test robot
drivers/media//platform/dwc/dw-hdmi-rx.c: In function '
dw_hdmi_registered':Quoted 1 line
Quoted 1 line
drivers/media//platform/dwc/dw-hdmi-rx.c:1452:9: error: implicit declaration of function '
v4l2_async_subnotifier_register' [-Werror=implicit-function-declaration]return v4l2_async_subnotifier_register(&dw_dev->sd, ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~drivers/media//platform/dwc/dw-hdmi-rx.c: In function '
dw_hdmi_unregistered':Quoted 1 line
Quoted 1 line
drivers/media//platform/dwc/dw-hdmi-rx.c:1462:2: error: implicit declaration of function '
v4l2_async_subnotifier_unregister' [-Werror=implicit-function-declaration]v4l2_async_subnotifier_unregister(&dw_dev->v4l2_notifier);
This one is expected. Required patch was not merged yet.
Quoted 9 lines
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/media//platform/dwc/dw-hdmi-rx.c: In function 'dw_hdmi_parse_dt':
drivers/media//platform/dwc/dw-hdmi-rx.c:1555:22: warning: unused variable 'notifier' [-Wunused-variable]
struct device_node *notifier, *phy_node, *np = dw_dev->of_node;
^~~~~~~~
drivers/media//platform/dwc/dw-hdmi-rx.c: In function 'dw_hdmi_rx_probe':
drivers/media//platform/dwc/dw-hdmi-rx.c:1765:1: warning: label 'err_phy' defined but not used [-Wunused-label]
err_phy:
^~~~~~~Quoted 3 lines
cc1: some warnings being treated as errors
These two is due to cec not being enabled. I hate to wrap this around a #ifdef but is either this or extract cec to a separate file, which I don't think is the best solution because the cec code is quite small.
Best regards,
Jose Miguel Abreu
Re: [PATCH v6 2/4] [media] platform: Add Synopsys Designware HDMI RX Controller Driver
Quoted 24 lines · Jose Abreu
Quoted 7 lines · kbuild test robot
drivers/media//platform/dwc/dw-hdmi-rx.c: In function '
dw_hdmi_registered':Quoted 1 line
Quoted 1 line
drivers/media//platform/dwc/dw-hdmi-rx.c:1452:9: error: implicit declaration of function '
v4l2_async_subnotifier_register' [-Werror=implicit-function-declaration]return v4l2_async_subnotifier_register(&dw_dev->sd, ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~drivers/media//platform/dwc/dw-hdmi-rx.c: In function '
dw_hdmi_unregistered':Quoted 1 line
Quoted 1 line
drivers/media//platform/dwc/dw-hdmi-rx.c:1462:2: error: implicit declaration of function '
v4l2_async_subnotifier_unregister' [-Werror=implicit-function-declaration]v4l2_async_subnotifier_unregister(&dw_dev->v4l2_notifier);This one is expected. Required patch was not merged yet.
Quoted 9 lines
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ drivers/media//platform/dwc/dw-hdmi-rx.c: In function 'dw_hdmi_parse_dt': drivers/media//platform/dwc/dw-hdmi-rx.c:1555:22: warning: unused variable 'notifier' [-Wunused-variable] struct device_node *notifier, *phy_node, *np = dw_dev->of_node; ^~~~~~~~ drivers/media//platform/dwc/dw-hdmi-rx.c: In function 'dw_hdmi_rx_probe': drivers/media//platform/dwc/dw-hdmi-rx.c:1765:1: warning: label 'err_phy' defined but not used [-Wunused-label] err_phy: ^~~~~~~Quoted 3 lines
cc1: some warnings being treated as errors
Quoted 4 lines
These two is due to cec not being enabled. I hate to wrap this around a #ifdef but is either this or extract cec to a separate file, which I don't think is the best solution because the cec code is quite small.
The notifier functions can also be used if CEC_NOTIFIER isn't set. See cec-notifier.h for the stub functions.
That will help somewhat.
Regards,
Hans
PATCH v6 3/4 MAINTAINERS: Add entry for Synopsys Designware HDMI drivers
In reply to Jose AbreuAdd a entry for Synopsys Designware HDMI Receivers drivers and phys.
--- MAINTAINERS | 7 +++++++ 1 file changed, 7 insertions(+)
MAINTAINERS +7 -0
@@ -11354,6 +11354,13 @@ L: [email protected] S: Supported F: drivers/net/ethernet/synopsys/ +SYNOPSYS DESIGNWARE HDMI RECEIVERS AND PHY DRIVERS +M: Jose Abreu <[email protected]> +L: [email protected] +S: Maintained +F: drivers/media/platform/dwc/* +F: include/media/dwc/* + SYNOPSYS DESIGNWARE I2C DRIVER M: Jarkko Nikula <[email protected]> R: Andy Shevchenko <[email protected]>
Signature · 1 line
1.9.1
PATCH v6 4/4 dt-bindings: media: Document Synopsys Designware HDMI RX
In reply to Jose AbreuDocument the bindings for the Synopsys Designware HDMI RX.
Changes from v4:
- Use "cfg" instead of "cfg-clk" (Rob)
- Change node names (Rob)
Changes from v3:
- Document the new DT bindings suggested by Sylwester
Changes from v2:
- Document edid-phandle property
--- .../devicetree/bindings/media/snps,dw-hdmi-rx.txt | 70 ++++++++++++++++++++++ 1 file changed, 70 insertions(+) create mode 100644 Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt
Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt new file +70 -0
@@ -0,0 +1,70 @@ +Synopsys DesignWare HDMI RX Decoder +=================================== + +This document defines device tree properties for the Synopsys DesignWare HDMI +RX Decoder (DWC HDMI RX). It doesn't constitute a device tree binding +specification by itself but is meant to be referenced by platform-specific +device tree bindings. + +When referenced from platform device tree bindings the properties defined in +this document are defined as follows. + +- compatible: Shall be "snps,dw-hdmi-rx". + +- reg: Memory mapped base address and length of the DWC HDMI RX registers. + +- interrupts: Reference to the DWC HDMI RX interrupt and 5v sense interrupt. + +- clocks: Phandle to the config clock block. + +- clock-names: Shall be "cfg". + +- edid-phandle: phandle to the EDID handler block. + +- #address-cells: Shall be 1. + +- #size-cells: Shall be 0. + +You also have to create a subnode for phy driver. Phy properties are as follows. + +- compatible: Shall be "snps,dw-hdmi-phy-e405". + +- reg: Shall be JTAG address of phy. + +- clocks: Phandle for cfg clock. + +- clock-names:Shall be "cfg". + +A sample binding is now provided. The compatible string is for a SoC which has +has a Synopsys DesignWare HDMI RX decoder inside. + +Example: + +dw_hdmi_soc: dw-hdmi-soc@0 { + compatible = "snps,dw-hdmi-soc"; + reg = <0x11c00 0x1000>; /* EDIDs */ + #address-cells = <1>; + #size-cells = <1>; + ranges; + + hdmi-rx@0 { + compatible = "snps,dw-hdmi-rx"; + reg = <0x0 0x10000>; + interrupts = <1 2>; + edid-phandle = <&dw_hdmi_soc>; + + clocks = <&dw_hdmi_refclk>; + clock-names = "cfg"; + + #address-cells = <1>; + #size-cells = <0>; + + hdmi-phy@fc { + compatible = "snps,dw-hdmi-phy-e405"; + reg = <0xfc>; + + clocks = <&dw_hdmi_refclk>; + clock-names = "cfg"; + }; + }; +};
Signature · 7 lines
1.9.1 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 4/4] dt-bindings: media: Document Synopsys Designware HDMI RX
Quoted 8 lines · Jose Abreu
Document the bindings for the Synopsys Designware HDMI RX.
--- .../devicetree/bindings/media/snps,dw-hdmi-rx.txt | 70 ++++++++++++++++++++++ 1 file changed, 70 insertions(+)create mode 100644
Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt
Could you make the DT binding documentation patch first patch in the series? Now checkpatch will complain about undocumented compatible string when the driver patches are applied alone.
Quoted 7 lines
Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt new file +0
-0
Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt line 10
Quoted 10 lines
Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt new file +10
-0@@ -0,0 +1,70 @@ +Synopsys DesignWare HDMI RX Decoder +=================================== + +This document defines device tree properties for the Synopsys DesignWare HDMI +RX Decoder (DWC HDMI RX). It doesn't constitute a device tree binding +specification by itself but is meant to be referenced by platform-specific +device tree bindings. + +When referenced from platform device tree bindings the properties defined in +this document are defined as follows.
It would be good to make it clear which properties are required and which are optional. And also to mention the properties below belong to the HDMI RX node.
Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt line 16
Quoted 5 lines
Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt new file +5
-0+- compatible: Shall be "snps,dw-hdmi-rx". + +- reg: Memory mapped base address and length of the DWC HDMI RX registers. + +- interrupts: Reference to the DWC HDMI RX interrupt and 5v sense interrupt.
s/5v/HDMI 5V ?
Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt line 22
Quoted 6 lines
Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt new file +6
-0+ +- clocks: Phandle to the config clock block. + +- clock-names: Shall be "cfg". + +- edid-phandle: phandle to the EDID handler block.
Could you make this property optional and when it is missing assume that device corresponding to the parent node of this node handles EDID? This way we could avoid having property pointing to the parent node.
Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt line 28
Quoted 5 lines
Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt new file +5
-0+- #address-cells: Shall be 1. + +- #size-cells: Shall be 0. + +You also have to create a subnode for phy driver. Phy properties are as follows.
s/phy driver. Phy/the PHY device. PHY ?
Might be also worth to make it explicit these are all required properties.
Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt line 32
Quoted 3 lines
Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt new file +3
-0+- compatible: Shall be "snps,dw-hdmi-phy-e405". + +- reg: Shall be JTAG address of phy.
s/phy/the PHY ?
Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt line 44
Quoted 11 lines
Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt new file +11
-0+- clocks: Phandle for cfg clock. + +- clock-names:Shall be "cfg". + +A sample binding is now provided. The compatible string is for a SoC which has +has a Synopsys DesignWare HDMI RX decoder inside. + +Example: + +dw_hdmi_soc: dw-hdmi-soc@0 { + compatible = "snps,dw-hdmi-soc";
Perhaps just make it
compatible = "...";
?
Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt line 45
Quoted 1 line
Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt new file +1
-0+ reg = <0x11c00 0x1000>; /* EDIDs */
This is not relevant and undocumented, will likely be part of documentation of other binding thus I'd suggest dropping this reg property.
Quoted 15 lines
Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt new file +15
-0+ #address-cells = <1>; + #size-cells = <1>; + ranges; + + hdmi-rx@0 { + compatible = "snps,dw-hdmi-rx"; + reg = <0x0 0x10000>; + interrupts = <1 2>; + edid-phandle = <&dw_hdmi_soc>; + + clocks = <&dw_hdmi_refclk>; + clock-names = "cfg"; + + #address-cells = <1>; + #size-cells = <0>;
Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt line 70
Quoted 10 lines
Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt new file +10
-0+ + hdmi-phy@fc { + compatible = "snps,dw-hdmi-phy-e405"; + reg = <0xfc>; + + clocks = <&dw_hdmi_refclk>; + clock-names = "cfg"; + }; + }; +};
Otherwise looks good. I'll likely not have comments to the other patches.
--
Regards,
Sylwester
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 4/4] dt-bindings: media: Document Synopsys Designware HDMI RX
Hi Sylwester,
Quoted 11 lines · Sylwester Nawrocki
Quoted 7 lines · Jose Abreu
Document the bindings for the Synopsys Designware HDMI RX.
Signed-off-by Jose Abreu <[email protected]>--- .../devicetree/bindings/media/snps,dw-hdmi-rx.txt | 70 ++++++++++++++++++++++ 1 file changed, 70 insertions(+)create mode 100644
Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txtCould you make the DT binding documentation patch first patch in the series? Now checkpatch will complain about undocumented compatible string when the driver patches are applied alone.
Sure.
Quoted 12 lines
Quoted 12 lines
Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt new file +4
-0@@ -0,0 +1,70 @@ +Synopsys DesignWare HDMI RX Decoder +=================================== + +This document defines device tree properties for the Synopsys DesignWare HDMI
Quoted 8 lines
Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt line 10
Quoted 6 lines
Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt new file +6
-0+RX Decoder (DWC HDMI RX). It doesn't constitute a device tree binding +specification by itself but is meant to be referenced by platform-specific +device tree bindings. + +When referenced from platform device tree bindings the properties defined in +this document are defined as follows.It would be good to make it clear which properties are required and which are optional. And also to mention the properties below belong to the HDMI RX node.
Ok.
Quoted 7 lines
Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt line 16
Quoted 6 lines
Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt new file +5
-0+- compatible: Shall be "snps,dw-hdmi-rx". + +- reg: Memory mapped base address and length of the DWC HDMI RX registers. + +- interrupts: Reference to the DWC HDMI RX interrupt and 5v sense interrupt.s/5v/HDMI 5V ?
Ok.
Quoted 10 lines
Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt line 22
Quoted 7 lines
Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt new file +6
-0+ +- clocks: Phandle to the config clock block. + +- clock-names: Shall be "cfg". + +- edid-phandle: phandle to the EDID handler block.Could you make this property optional and when it is missing assume that device corresponding to the parent node of this node handles EDID? This way we could avoid having property pointing to the parent node.
Hmm, this is for the CEC notifier. Do you mean I should grab the parent device for the notifier? This property is already optional if cec is not enabled though.
Quoted 9 lines
Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt line 28
Quoted 6 lines
Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt new file +5
-0+- #address-cells: Shall be 1. + +- #size-cells: Shall be 0. + +You also have to create a subnode for phy driver. Phy properties are as follows.s/phy driver. Phy/the PHY device. PHY ?
Might be also worth to make it explicit these are all required properties.
Ok.
Quoted 5 lines
Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt line 32
Quoted 4 lines
Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt new file +3
-0+- compatible: Shall be "snps,dw-hdmi-phy-e405". + +- reg: Shall be JTAG address of phy.s/phy/the PHY ?
Ok.
Quoted 14 lines
Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt line 44
Quoted 12 lines
Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt new file +11
-0+- clocks: Phandle for cfg clock. + +- clock-names:Shall be "cfg". + +A sample binding is now provided. The compatible string is for a SoC which has +has a Synopsys DesignWare HDMI RX decoder inside. + +Example: + +dw_hdmi_soc: dw-hdmi-soc@0 { + compatible = "snps,dw-hdmi-soc";Perhaps just make it
Quoted 2 lines
compatible = "...";?
Yeah, probably its better.
Quoted 4 lines
Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt line 45
Quoted 2 lines
Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt new file +1
-0+ reg = <0x11c00 0x1000>; /* EDIDs */This is not relevant and undocumented, will likely be part of documentation of other binding thus I'd suggest dropping this reg property.
Ok.
Quoted 19 lines
Quoted 19 lines
Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt new file +18
-0+ #address-cells = <1>; + #size-cells = <1>; + ranges; + + hdmi-rx@0 { + compatible = "snps,dw-hdmi-rx"; + reg = <0x0 0x10000>; + interrupts = <1 2>; + edid-phandle = <&dw_hdmi_soc>; + + clocks = <&dw_hdmi_refclk>; + clock-names = "cfg"; + + #address-cells = <1>; + #size-cells = <0>; + + hdmi-phy@fc { + compatible = "snps,dw-hdmi-phy-e405";
Quoted 8 lines
Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt line 70
Quoted 7 lines
Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt new file +7
-0+ reg = <0xfc>; + + clocks = <&dw_hdmi_refclk>; + clock-names = "cfg"; + }; + }; +};Otherwise looks good. I'll likely not have comments to the other patches.
Thanks for the review!
Best regards,
Jose Miguel Abreu
Quoted 5 lines
--
Regards,
Sylwester
Re: [PATCH v6 4/4] dt-bindings: media: Document Synopsys Designware HDMI RX
Quoted 9 lines · Jose Abreu
Quoted 5 lines
Quoted 1 line
+- edid-phandle: phandle to the EDID handler block.
Could you make this property optional and when it is missing assume that device corresponding to the parent node of this node handles EDID? This way we could avoid having property pointing to the parent node.
Hmm, this is for the CEC notifier. Do you mean I should grab the parent device for the notifier? This property is already optional if cec is not enabled though.
Yes, device associated with the parent node. Something like:
- edid-phandle - phandle to the EDID handler block; if this property is not specified it is assumed that EDID is handled by device described by parent node of the HDMI RX node
Not sure if it is any better than always requiring edid-phandle property, even when it is pointing to the parent node. We would need a DT maintainer's opinion on that.
--
Thanks,
Sylwester
Re: [PATCH v6 4/4] dt-bindings: media: Document Synopsys Designware HDMI RX
Quoted 11 lines · Sylwester Nawrocki
Quoted 7 lines · Jose Abreu
Quoted 4 lines
Quoted 1 line
+- edid-phandle: phandle to the EDID handler block.
Could you make this property optional and when it is missing assume that device corresponding to the parent node of this node handles EDID? This way we could avoid having property pointing to the parent node.
Hmm, this is for the CEC notifier. Do you mean I should grab the parent device for the notifier? This property is already optional if cec is not enabled though.
Yes, device associated with the parent node. Something like:
Quoted 7 lines
- edid-phandle - phandle to the EDID handler block; if this property is not specified it is assumed that EDID is handled by device described by parent node of the HDMI RX node
Not sure if it is any better than always requiring edid-phandle property, even when it is pointing to the parent node. We would need a DT maintainer's opinion on that.
I will change and resend. I also have to fix a kbuild error when cec is not enabled.
Best regards,
Jose Miguel Abreu
Quoted 4 lines
--
Thanks,
Sylwester