Re: [PATCH 1/2] drm/bridge: add Silicon Image SiI9234 driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




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
<t.stanislaws@xxxxxxxxxxx>, which was initially developed by:
Adam Hampson <ahampson@xxxxxxxxxxxxxxx>
Erik Gilling <konkers@xxxxxxxxxxx>
Shankar Bandal <shankar.b@xxxxxxxxxxx>
Dharam Kumar <dharam.kr@xxxxxxxxxxx>

Signed-off-by: Maciej Purski <m.purski@xxxxxxxxxxx>
---

   .../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 0000000..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

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux