Re: [PATCH 1/2] drm/bridge: add Silicon Image SiI9234 driver
Hi Maciej,
[auto build test ERROR on drm/drm-next]
[also build test ERROR on v4.13-rc4 next-20170811]
[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/Maciej-Purski/add-Silicon-Image-SiI9234-driver/20170803-200255
base: git://people.freedesktop.org/~airlied/linux.git drm-next
config: i386-randconfig-h0-08130402 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All errors (new ones prefixed by >>):
drivers/gpu//drm/bridge/sii9234.c: In function 'sii9234_mode_valid':
drivers/gpu//drm/bridge/sii9234.c:909:18: warning: unused variable 'ctx'
[-Wunused-variable]
struct sii9234 *ctx = bridge_to_sii9234(bridge);
^~~
drivers/gpu//drm/bridge/sii9234.c: In function 'sii9234_mhl_tx_i2c_probe':
>> drivers/gpu//drm/bridge/sii9234.c:971:13: error: 'struct drm_bridge' has no
>> member named 'of_node'
ctx->bridge.of_node = dev->of_node;
^
In file included from include/uapi/linux/stddef.h:1:0,
from include/linux/stddef.h:4,
from include/uapi/linux/posix_types.h:4,
from include/uapi/linux/types.h:13,
from include/linux/types.h:5,
from include/drm/bridge/mhl.h:18,
from drivers/gpu//drm/bridge/sii9234.c:26:
drivers/gpu//drm/bridge/sii9234.c: At top level:
include/linux/compiler.h:162:4: warning: '__f' is static but declared in
inline function 'strcpy' which is not static
__f = { \
^
include/linux/compiler.h:154:23: note: in expansion of macro '__trace_if'
#define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
^~
include/linux/string.h:390:2: note: in expansion of macro 'if'
if (p_size == (size_t)-1 && q_size == (size_t)-1)
^~
include/linux/compiler.h:162:4: warning: '__f' is static but declared in
inline function 'kmemdup' which is not static
__f = { \
^
include/linux/compiler.h:154:23: note: in expansion of macro '__trace_if'
#define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
^~
include/linux/string.h:380:2: note: in expansion of macro 'if'
if (p_size < size)
^~
include/linux/compiler.h:162:4: warning: '__f' is static but declared in
inline function 'kmemdup' which is not static
__f = { \
^
include/linux/compiler.h:154:23: note: in expansion of macro '__trace_if'
#define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
^~
include/linux/string.h:378:2: note: in expansion of macro 'if'
if (__builtin_constant_p(size) && p_size < size)
^~
include/linux/compiler.h:162:4: warning: '__f' is static but declared in
inline function 'memchr_inv' which is not static
__f = { \
^
include/linux/compiler.h:154:23: note: in expansion of macro '__trace_if'
#define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
^~
include/linux/string.h:369:2: note: in expansion of macro 'if'
if (p_size < size)
^~
include/linux/compiler.h:162:4: warning: '__f' is static but declared in
inline function 'memchr_inv' which is not static
__f = { \
^
include/linux/compiler.h:154:23: note: in expansion of macro '__trace_if'
#define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
^~
include/linux/string.h:367:2: note: in expansion of macro 'if'
if (__builtin_constant_p(size) && p_size < size)
^~
include/linux/compiler.h:162:4: warning: '__f' is static but declared in
inline function 'memchr' which is not static
__f = { \
^
include/linux/compiler.h:154:23: note: in expansion of macro '__trace_if'
#define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
^~
include/linux/string.h:358:2: note: in expansion of macro 'if'
if (p_size < size)
^~
include/linux/compiler.h:162:4: warning: '__f' is static but declared in
inline function 'memchr' which is not static
__f = { \
^
include/linux/compiler.h:154:23: note: in expansion of macro '__trace_if'
#define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
^~
include/linux/string.h:356:2: note: in expansion of macro 'if'
if (__builtin_constant_p(size) && p_size < size)
^~
include/linux/compiler.h:162:4: warning: '__f' is static but declared in
inline function 'memcmp' which is not static
__f = { \
^
include/linux/compiler.h:154:23: note: in expansion of macro '__trace_if'
#define if(cond, ...
Re: [PATCH 1/2] drm/bridge: add Silicon Image SiI9234 driver
Hi Marek, (now CC'ing Mark Brown with his correct e-mail address) On Friday 11 Aug 2017 09:00:00 Marek Szyprowski wrote: > On 2017-08-10 16:51, Laurent Pinchart wrote: > > On Friday 04 Aug 2017 08:55:55 Marek Szyprowski wrote: > >> Hi Laurent, > >> > >> Thanks for your detailed comments. Maciej resurrected some orphaned code, > >> which is still useful today (Tomasz has left Samsung a few years ago). > >> I'm not sure we will be able to answer all your questions without deep > >> investigation, especially about the driver operation details, but we > >> will try. > > > > Thank you. > > > >> On 2017-08-03 12:24, Laurent Pinchart wrote: > >>> On Thursday 03 Aug 2017 09:45:22 Maciej Purski wrote: > SiI9234 transmitter converts eTMDS/HDMI signal to MHL 1.0. > It is controlled via I2C bus. Its interaction with other > devices in video pipeline is performed mainly on HW level. > The only interaction it does on device driver level is > filtering-out unsupported video modes, it exposes drm_bridge > interface to perform this operation. > > This patch is based on the code refactored by Tomasz Stanislawski > , which was initially developed by: > Adam Hampson > Erik Gilling > Shankar Bandal > Dharam Kumar > > Signed-off-by: Maciej Purski > --- > > .../devicetree/bindings/display/bridge/sii9234.txt | 20 + > drivers/gpu/drm/bridge/Kconfig |8 + > drivers/gpu/drm/bridge/Makefile|1 + > drivers/gpu/drm/bridge/sii9234.c | 1019 ++ > 4 files changed, 1048 insertions(+) > create mode 100644 > > Documentation/devicetree/bindings/display/bridge/sii9234.txt create > mode > 100644 drivers/gpu/drm/bridge/sii9234.c > > diff --git > a/Documentation/devicetree/bindings/display/bridge/sii9234.txt > b/Documentation/devicetree/bindings/display/bridge/sii9234.txt new file > mode 100644 > index 000..2cdf286 > --- /dev/null > +++ b/Documentation/devicetree/bindings/display/bridge/sii9234.txt > >>> > >>> DT reviewers might ask you to submit DT bindings as a separate patch. > >>> > @@ -0,0 +1,20 @@ > +SiI9234 Mobile HD Link Transmitter > + > +Required properties: > +- compatible : "sil,sii9234". > +- reg : I2C address for TPI interface, use 0x39 > +- vcc-supply : regulator that supplies the chip > >>> > >>> Is there a single power supply only ? Transceivers usually have at least > >>> one digital and one analog power supply. > >> > >> Acording to the schematic there are four power supplies used for SII9234 > >> MHL chip in Trats2 board: VSIL_1.2A, VSIL_1.2C, VCC_3.3V_MHL and > >> VCC_1.8V_MHL. First two are derived directly from VSIL_1.2 signal, which > >> is modeled as a fixed regulator. The latter two are derived directly from > >> VBAT using some level converter, which is controlled by the same GPIO pin > >> as VSIL fixed regulator. Any idea how this should be represented better > >> in device tree instead of having single vcc-supply? > > > > Without access to the documentation I can only guess, but it looks like > > VSIL_1.2A and VSIL_1.2C are supposed to be powered from the same power > > supply rail, possibly with different filters. I think they can be grouped > > together from a DT binding point of view. The last two supplies seem > > independent. We should thus probably model this as three separate > > supplies. > > Okay, I see no problem adding support for all those three supplies, but > I was wondering how to model them in the device tree, because from the > software perspective ALL power supplies needed by this chip are enabled by a > single GPIO line switch. > > I see 3 possible solutions: > 1. Keep only single vcc supply for now and use fixed gpio regulator for it > as a provider. Add a comment that it fact it provides 3 different power > signals. > 2. Extend fixed gpio regulator driver and bindings so it will be possible to > have more than one fixed regulator controlled by the same gpio pin. > 3. Model VCC_3.3V_MHL and VCC_1.8V_MHL providers as "vctrl-regulator" and > use this VSIL_1.2 as control voltage for them. > > Which one do you prefer? 2 would be best I think, but that's more work. Mark, what do you think ? > > It would be useful to check in the SII9234 datasheet what power sequence > > the chip expects. Is there any chance you could find that document ? > > We have access only to the parts of the SII9234 documentation now and > there is no > such information there. > > +- interrupts, interrupt-parent: interrupt specifier of INT pin > +- reset-gpios: gpio specifier of RESET pin > >>> > >>> Is this mandatory to connect the reset pin to the SoC ? > >> > >> IMHO yes, the chip has to be reset during the initialization procedure > >> and doesn't work properly without reset. > > > > OK,
Re: [PATCH 1/2] drm/bridge: add Silicon Image SiI9234 driver
Hi Marek, (CC'ing Mark Brown) On Friday 11 Aug 2017 09:00:00 Marek Szyprowski wrote: > On 2017-08-10 16:51, Laurent Pinchart wrote: > > On Friday 04 Aug 2017 08:55:55 Marek Szyprowski wrote: > >> Hi Laurent, > >> > >> Thanks for your detailed comments. Maciej resurrected some orphaned code, > >> which is still useful today (Tomasz has left Samsung a few years ago). > >> I'm not sure we will be able to answer all your questions without deep > >> investigation, especially about the driver operation details, but we > >> will try. > > > > Thank you. > > > >> On 2017-08-03 12:24, Laurent Pinchart wrote: > >>> On Thursday 03 Aug 2017 09:45:22 Maciej Purski wrote: > SiI9234 transmitter converts eTMDS/HDMI signal to MHL 1.0. > It is controlled via I2C bus. Its interaction with other > devices in video pipeline is performed mainly on HW level. > The only interaction it does on device driver level is > filtering-out unsupported video modes, it exposes drm_bridge > interface to perform this operation. > > This patch is based on the code refactored by Tomasz Stanislawski > , which was initially developed by: > Adam Hampson > Erik Gilling > Shankar Bandal > Dharam Kumar > > Signed-off-by: Maciej Purski > --- > > .../devicetree/bindings/display/bridge/sii9234.txt | 20 + > drivers/gpu/drm/bridge/Kconfig |8 + > drivers/gpu/drm/bridge/Makefile|1 + > drivers/gpu/drm/bridge/sii9234.c | 1019 ++ > 4 files changed, 1048 insertions(+) > create mode 100644 > > Documentation/devicetree/bindings/display/bridge/sii9234.txt create > mode > 100644 drivers/gpu/drm/bridge/sii9234.c > > diff --git > a/Documentation/devicetree/bindings/display/bridge/sii9234.txt > b/Documentation/devicetree/bindings/display/bridge/sii9234.txt new file > mode 100644 > index 000..2cdf286 > --- /dev/null > +++ b/Documentation/devicetree/bindings/display/bridge/sii9234.txt > >>> > >>> DT reviewers might ask you to submit DT bindings as a separate patch. > >>> > @@ -0,0 +1,20 @@ > +SiI9234 Mobile HD Link Transmitter > + > +Required properties: > +- compatible : "sil,sii9234". > +- reg : I2C address for TPI interface, use 0x39 > +- vcc-supply : regulator that supplies the chip > >>> > >>> Is there a single power supply only ? Transceivers usually have at least > >>> one digital and one analog power supply. > >> > >> Acording to the schematic there are four power supplies used for SII9234 > >> MHL chip in Trats2 board: VSIL_1.2A, VSIL_1.2C, VCC_3.3V_MHL and > >> VCC_1.8V_MHL. First two are derived directly from VSIL_1.2 signal, which > >> is modeled as a fixed regulator. The latter two are derived directly from > >> VBAT using some level converter, which is controlled by the same GPIO pin > >> as VSIL fixed regulator. Any idea how this should be represented better > >> in device tree instead of having single vcc-supply? > > > > Without access to the documentation I can only guess, but it looks like > > VSIL_1.2A and VSIL_1.2C are supposed to be powered from the same power > > supply rail, possibly with different filters. I think they can be grouped > > together from a DT binding point of view. The last two supplies seem > > independent. We should thus probably model this as three separate > > supplies. > > Okay, I see no problem adding support for all those three supplies, but > I was wondering how to model them in the device tree, because from the > software perspective ALL power supplies needed by this chip are enabled by a > single GPIO line switch. > > I see 3 possible solutions: > 1. Keep only single vcc supply for now and use fixed gpio regulator for it > as a provider. Add a comment that it fact it provides 3 different power > signals. > 2. Extend fixed gpio regulator driver and bindings so it will be possible to > have more than one fixed regulator controlled by the same gpio pin. > 3. Model VCC_3.3V_MHL and VCC_1.8V_MHL providers as "vctrl-regulator" and > use this VSIL_1.2 as control voltage for them. > > Which one do you prefer? 2 would be best I think, but that's more work. Mark, what do you think ? > > It would be useful to check in the SII9234 datasheet what power sequence > > the chip expects. Is there any chance you could find that document ? > > We have access only to the parts of the SII9234 documentation now and > there is no > such information there. > > +- interrupts, interrupt-parent: interrupt specifier of INT pin > +- reset-gpios: gpio specifier of RESET pin > >>> > >>> Is this mandatory to connect the reset pin to the SoC ? > >> > >> IMHO yes, the chip has to be reset during the initialization procedure > >> and doesn't work properly without reset. > > > > OK, I have no issue making the property
Re: [PATCH 1/2] drm/bridge: add Silicon Image SiI9234 driver
Hi Laurent, On 2017-08-10 16:51, Laurent Pinchart wrote: Hi Marek, On Friday 04 Aug 2017 08:55:55 Marek Szyprowski wrote: Hi Laurent, Thanks for your detailed comments. Maciej resurrected some orphaned code, which is still useful today (Tomasz has left Samsung a few years ago). I'm not sure we will be able to answer all your questions without deep investigation, especially about the driver operation details, but we will try. Thank you. On 2017-08-03 12:24, Laurent Pinchart wrote: On Thursday 03 Aug 2017 09:45:22 Maciej Purski wrote: SiI9234 transmitter converts eTMDS/HDMI signal to MHL 1.0. It is controlled via I2C bus. Its interaction with other devices in video pipeline is performed mainly on HW level. The only interaction it does on device driver level is filtering-out unsupported video modes, it exposes drm_bridge interface to perform this operation. This patch is based on the code refactored by Tomasz Stanislawski , which was initially developed by: Adam Hampson Erik Gilling Shankar Bandal Dharam Kumar Signed-off-by: Maciej Purski --- .../devicetree/bindings/display/bridge/sii9234.txt | 20 + drivers/gpu/drm/bridge/Kconfig |8 + drivers/gpu/drm/bridge/Makefile|1 + drivers/gpu/drm/bridge/sii9234.c | 1019 + 4 files changed, 1048 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/bridge/sii9234.txt create mode 100644 drivers/gpu/drm/bridge/sii9234.c diff --git a/Documentation/devicetree/bindings/display/bridge/sii9234.txt b/Documentation/devicetree/bindings/display/bridge/sii9234.txt new file mode 100644 index 000..2cdf286 --- /dev/null +++ b/Documentation/devicetree/bindings/display/bridge/sii9234.txt DT reviewers might ask you to submit DT bindings as a separate patch. @@ -0,0 +1,20 @@ +SiI9234 Mobile HD Link Transmitter + +Required properties: +- compatible : "sil,sii9234". +- reg : I2C address for TPI interface, use 0x39 +- vcc-supply : regulator that supplies the chip Is there a single power supply only ? Transceivers usually have at least one digital and one analog power supply. Acording to the schematic there are four power supplies used for SII9234 MHL chip in Trats2 board: VSIL_1.2A, VSIL_1.2C, VCC_3.3V_MHL and VCC_1.8V_MHL. First two are derived directly from VSIL_1.2 signal, which is modeled as a fixed regulator. The latter two are derived directly from VBAT using some level converter, which is controlled by the same GPIO pin as VSIL fixed regulator. Any idea how this should be represented better in device tree instead of having single vcc-supply? Without access to the documentation I can only guess, but it looks like VSIL_1.2A and VSIL_1.2C are supposed to be powered from the same power supply rail, possibly with different filters. I think they can be grouped together from a DT binding point of view. The last two supplies seem independent. We should thus probably model this as three separate supplies. Okay, I see no problem adding support for all those three supplies, but I was wondering how to model them in the device tree, because from the software perspective ALL power supplies needed by this chip are enabled by a single GPIO line switch. I see 3 possible solutions: 1. Keep only single vcc supply for now and use fixed gpio regulator for it as a provider. Add a comment that it fact it provides 3 different power signals. 2. Extend fixed gpio regulator driver and bindings so it will be possible to have more than one fixed regulator controlled by the same gpio pin. 3. Model VCC_3.3V_MHL and VCC_1.8V_MHL providers as "vctrl-regulator" and use this VSIL_1.2 as control voltage for them. Which one do you prefer? It would be useful to check in the SII9234 datasheet what power sequence the chip expects. Is there any chance you could find that document ? We have access only to the parts of the SII9234 documentation now and there is no such information there. +- interrupts, interrupt-parent: interrupt specifier of INT pin +- reset-gpios: gpio specifier of RESET pin Is this mandatory to connect the reset pin to the SoC ? IMHO yes, the chip has to be reset during the initialization procedure and doesn't work properly without reset. OK, I have no issue making the property mandatory then. Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] drm/bridge: add Silicon Image SiI9234 driver
Hi Marek, On Friday 04 Aug 2017 08:55:55 Marek Szyprowski wrote: > Hi Laurent, > > Thanks for your detailed comments. Maciej resurrected some orphaned code, > which is still useful today (Tomasz has left Samsung a few years ago). > I'm not sure we will be able to answer all your questions without deep > investigation, especially about the driver operation details, but we > will try. Thank you. > On 2017-08-03 12:24, Laurent Pinchart wrote: > > On Thursday 03 Aug 2017 09:45:22 Maciej Purski wrote: > >> SiI9234 transmitter converts eTMDS/HDMI signal to MHL 1.0. > >> It is controlled via I2C bus. Its interaction with other > >> devices in video pipeline is performed mainly on HW level. > >> The only interaction it does on device driver level is > >> filtering-out unsupported video modes, it exposes drm_bridge > >> interface to perform this operation. > >> > >> This patch is based on the code refactored by Tomasz Stanislawski > >> , which was initially developed by: > >> Adam Hampson > >> Erik Gilling > >> Shankar Bandal > >> Dharam Kumar > >> > >> Signed-off-by: Maciej Purski > >> --- > >> > >> .../devicetree/bindings/display/bridge/sii9234.txt | 20 + > >> drivers/gpu/drm/bridge/Kconfig |8 + > >> drivers/gpu/drm/bridge/Makefile|1 + > >> drivers/gpu/drm/bridge/sii9234.c | 1019 + > >> 4 files changed, 1048 insertions(+) > >> create mode 100644 > >> > >> Documentation/devicetree/bindings/display/bridge/sii9234.txt create mode > >> 100644 drivers/gpu/drm/bridge/sii9234.c > >> > >> diff --git a/Documentation/devicetree/bindings/display/bridge/sii9234.txt > >> b/Documentation/devicetree/bindings/display/bridge/sii9234.txt new file > >> mode 100644 > >> index 000..2cdf286 > >> --- /dev/null > >> +++ b/Documentation/devicetree/bindings/display/bridge/sii9234.txt > > > > DT reviewers might ask you to submit DT bindings as a separate patch. > > > >> @@ -0,0 +1,20 @@ > >> +SiI9234 Mobile HD Link Transmitter > >> + > >> +Required properties: > >> +- compatible : "sil,sii9234". > >> +- reg : I2C address for TPI interface, use 0x39 > >> +- vcc-supply : regulator that supplies the chip > > > > Is there a single power supply only ? Transceivers usually have at least > > one digital and one analog power supply. > > Acording to the schematic there are four power supplies used for SII9234 > MHL chip in Trats2 board: VSIL_1.2A, VSIL_1.2C, VCC_3.3V_MHL and > VCC_1.8V_MHL. First two are derived directly from VSIL_1.2 signal, which is > modeled as a fixed regulator. The latter two are derived directly from VBAT > using some level converter, which is controlled by the same GPIO pin as VSIL > fixed regulator. Any idea how this should be represented better in device > tree instead of having single vcc-supply? Without access to the documentation I can only guess, but it looks like VSIL_1.2A and VSIL_1.2C are supposed to be powered from the same power supply rail, possibly with different filters. I think they can be grouped together from a DT binding point of view. The last two supplies seem independent. We should thus probably model this as three separate supplies. It would be useful to check in the SII9234 datasheet what power sequence the chip expects. Is there any chance you could find that document ? > >> +- interrupts, interrupt-parent: interrupt specifier of INT pin > >> +- reset-gpios: gpio specifier of RESET pin > > > > Is this mandatory to connect the reset pin to the SoC ? > > IMHO yes, the chip has to be reset during the initialization procedure > and doesn't work properly without reset. OK, I have no issue making the property mandatory then. -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] drm/bridge: add Silicon Image SiI9234 driver
Hi Laurent, Thanks for your detailed comments. Maciej resurrected some orphaned code, which is still useful today (Tomasz has left Samsung a few years ago). I'm not sure we will be able to answer all your questions without deep investigation, especially about the driver operation details, but we will try. On 2017-08-03 12:24, Laurent Pinchart wrote: Hi Maciej, Thank you for the patch. On Thursday 03 Aug 2017 09:45:22 Maciej Purski wrote: SiI9234 transmitter converts eTMDS/HDMI signal to MHL 1.0. It is controlled via I2C bus. Its interaction with other devices in video pipeline is performed mainly on HW level. The only interaction it does on device driver level is filtering-out unsupported video modes, it exposes drm_bridge interface to perform this operation. This patch is based on the code refactored by Tomasz Stanislawski , which was initially developed by: Adam Hampson Erik Gilling Shankar Bandal Dharam Kumar Signed-off-by: Maciej Purski --- .../devicetree/bindings/display/bridge/sii9234.txt | 20 + drivers/gpu/drm/bridge/Kconfig |8 + drivers/gpu/drm/bridge/Makefile|1 + drivers/gpu/drm/bridge/sii9234.c | 1019 + 4 files changed, 1048 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/bridge/sii9234.txt create mode 100644 drivers/gpu/drm/bridge/sii9234.c diff --git a/Documentation/devicetree/bindings/display/bridge/sii9234.txt b/Documentation/devicetree/bindings/display/bridge/sii9234.txt new file mode 100644 index 000..2cdf286 --- /dev/null +++ b/Documentation/devicetree/bindings/display/bridge/sii9234.txt DT reviewers might ask you to submit DT bindings as a separate patch. @@ -0,0 +1,20 @@ +SiI9234 Mobile HD Link Transmitter + +Required properties: +- compatible : "sil,sii9234". +- reg : I2C address for TPI interface, use 0x39 +- vcc-supply : regulator that supplies the chip Is there a single power supply only ? Transceivers usually have at least one digital and one analog power supply. Acording to the schematic there are four power supplies used for SII9234 MHL chip in Trats2 board: VSIL_1.2A, VSIL_1.2C, VCC_3.3V_MHL and VCC_1.8V_MHL. First two are derived directly from VSIL_1.2 signal, which is modeled as a fixed regulator. The latter two are derived directly from VBAT using some level converter, which is controlled by the same GPIO pin as VSIL fixed regulator. Any idea how this should be represented better in device tree instead of having single vcc-supply? +- interrupts, interrupt-parent: interrupt specifier of INT pin +- reset-gpios: gpio specifier of RESET pin Is this mandatory to connect the reset pin to the SoC ? IMHO yes, the chip has to be reset during the initialization procedure and doesn't work properly without reset. > [...] Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] drm/bridge: add Silicon Image SiI9234 driver
Hi Maciej, [auto build test WARNING on drm/drm-next] [also build test WARNING on v4.13-rc3 next-20170803] [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/Maciej-Purski/add-Silicon-Image-SiI9234-driver/20170803-200255 base: git://people.freedesktop.org/~airlied/linux.git drm-next coccinelle warnings: (new ones prefixed by >>) >> drivers/gpu/drm/bridge/sii9234.c:1010:3-8: No need to set .owner here. The >> core will do it. Please review and possibly fold the followup patch. --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] drm/bridge: add Silicon Image SiI9234 driver
Hi Maciej,
Thank you for the patch.
On Thursday 03 Aug 2017 09:45:22 Maciej Purski wrote:
> SiI9234 transmitter converts eTMDS/HDMI signal to MHL 1.0.
> It is controlled via I2C bus. Its interaction with other
> devices in video pipeline is performed mainly on HW level.
> The only interaction it does on device driver level is
> filtering-out unsupported video modes, it exposes drm_bridge
> interface to perform this operation.
>
> This patch is based on the code refactored by Tomasz Stanislawski
> , which was initially developed by:
> Adam Hampson
> Erik Gilling
> Shankar Bandal
> Dharam Kumar
>
> Signed-off-by: Maciej Purski
> ---
> .../devicetree/bindings/display/bridge/sii9234.txt | 20 +
> drivers/gpu/drm/bridge/Kconfig |8 +
> drivers/gpu/drm/bridge/Makefile|1 +
> drivers/gpu/drm/bridge/sii9234.c | 1019 +
> 4 files changed, 1048 insertions(+)
> create mode 100644
> Documentation/devicetree/bindings/display/bridge/sii9234.txt create mode
> 100644 drivers/gpu/drm/bridge/sii9234.c
>
> diff --git a/Documentation/devicetree/bindings/display/bridge/sii9234.txt
> b/Documentation/devicetree/bindings/display/bridge/sii9234.txt new file
> mode 100644
> index 000..2cdf286
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/bridge/sii9234.txt
DT reviewers might ask you to submit DT bindings as a separate patch.
> @@ -0,0 +1,20 @@
> +SiI9234 Mobile HD Link Transmitter
> +
> +Required properties:
> +- compatible : "sil,sii9234".
> +- reg : I2C address for TPI interface, use 0x39
> +- vcc-supply : regulator that supplies the chip
Is there a single power supply only ? Transceivers usually have at least one
digital and one analog power supply.
> +- interrupts, interrupt-parent: interrupt specifier of INT pin
> +- reset-gpios: gpio specifier of RESET pin
Is this mandatory to connect the reset pin to the SoC ?
You should use the OF graph DT bindings (a.k.a. ports) to describe the data
connections with the rest of the system. I'm not familiar with this chip, but
I assume it should have one input port connected to the SoC and one output
port connected to an HDMI connector DT node.
> +Additional compatible properties are also allowed.
How so ? :-)
> +
> +Example:
> + sii9234@39 {
> + compatible = "sil,sii9234";
> + reg = <0x39>;
> + vcc-supply = <&vsil>;
> + reset-gpios = <&gpf3 4 GPIO_ACTIVE_HIGH>;
> + interrupt-parent = <&gpf3>;
> + interrupts = <5 IRQ_TYPE_LEVEL_HIGH>;
> + };
> diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
> index adf9ae0..f5784e5 100644
> --- a/drivers/gpu/drm/bridge/Kconfig
> +++ b/drivers/gpu/drm/bridge/Kconfig
> @@ -100,6 +100,14 @@ config DRM_TI_TFP410
> ---help---
> Texas Instruments TFP410 DVI/HDMI Transmitter driver
>
> +config DRM_SII9234
> + tristate "Silicon Image SII9234 Driver"
> + depends on I2C
You can depend on OF too as the driver doesn't currently support any other
method.
> + help
> + Say Y here if you want support for the MHL interface.
> + It is an I2C driver, that detects connection of MHL bridge
> + and starts encapsulation of HDMI signal.
> +
> source "drivers/gpu/drm/bridge/analogix/Kconfig"
>
> source "drivers/gpu/drm/bridge/adv7511/Kconfig"
> diff --git a/drivers/gpu/drm/bridge/Makefile
> b/drivers/gpu/drm/bridge/Makefile index defcf1e..e3d5eb0 100644
> --- a/drivers/gpu/drm/bridge/Makefile
> +++ b/drivers/gpu/drm/bridge/Makefile
> @@ -6,6 +6,7 @@ obj-$(CONFIG_DRM_NXP_PTN3460) += nxp-ptn3460.o
> obj-$(CONFIG_DRM_PARADE_PS8622) += parade-ps8622.o
> obj-$(CONFIG_DRM_SIL_SII8620) += sil-sii8620.o
> obj-$(CONFIG_DRM_SII902X) += sii902x.o
> +obj-$(CONFIG_DRM_SII9234) += sii9234.o
> obj-$(CONFIG_DRM_TOSHIBA_TC358767) += tc358767.o
> obj-$(CONFIG_DRM_ANALOGIX_DP) += analogix/
> obj-$(CONFIG_DRM_I2C_ADV7511) += adv7511/
> diff --git a/drivers/gpu/drm/bridge/sii9234.c
> b/drivers/gpu/drm/bridge/sii9234.c new file mode 100644
> index 000..9c436e7
> --- /dev/null
> +++ b/drivers/gpu/drm/bridge/sii9234.c
> @@ -0,0 +1,1019 @@
> +/*
> + * Copyright (C) 2014 Samsung Electronics
> + *
> + * Author: Tomasz Stanislawski
> + *
> + * Based on sii9234 driver created by:
> + *Adam Hampson
> + *Erik Gilling
> + *Shankar Bandal
> + *Dharam Kumar
> + *
> + * 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.
> + *
> + * You
