Hi Sylwester,
On 19-06-2017 22:18, Sylwester Nawrocki wrote:
> On 06/19/2017 11:01 AM, Jose Abreu wrote:
>> Using fixed-clock was already in my todo list. Regarding phy I
>> need to pass pdata so that the callbacks between controller and
>> phy are established. I also need to make sure that phy driver
>> will be loaded by the controller driver. Hmm, and also address of
>> the phy on th JTAG bus is fed to the controller driver not to the
>> phy driver. Maybe leave the property as is (the
>> "snps,hdmi-phy-jtag-addr") or parse it from the phy node?
> I think the RX controller can parse it's child phy node to retrieve JTAG
> address from the reg property. That seems better than creating custom
> property for device address on the bus.
Ok, will do that.
>
> Does the PHY support any other type of control bus, e.g. I2C or SPI?
It supports I2C but its all done by controller regbank.
>
>> I also need to pass pdata to the controller driver (the callbacks
>> for 5v handling) which are agnostic of the controller. These
> Is this about detecting +5V coming from the HDMI connector? Or some
> other voltage supply?
Yes, 5v signal from HDMI source.
>
>> reasons prevented me from adding compatible strings to both
>> drivers and just use a wrapper driver instead. This way i do
> If you add struct of_device_id instance to your module and define
> MODULE_DEVICE_ALIAS(of, ...) there, it will allow the module to be loaded
> when device with matching compatible string is created in the kernel.
> User space is notified with uevent about device creation.
Hmm, I used another approach. I have a list of supported phys in
the controller driver and the corresponding driver name, then I
just match the node, get the driver name and do a request module
the of_platform_populate. Maybe its not the best option?
>
>> "modprobe wrapper_driver" and I get all the drivers loaded via
>> request_module(). Still, I like your approach much better. I saw
>> that I can pass pdata using of_platform_populate, could you
>> please confirm if I can still maintain this architecture (i.e.
>> prevent modules from loading until I get all the chain setup)?
> You could try to pass platform data this way, that should work. But
> I doubt it's the right directions, I would rather see things done
> in the V4L2 layer.
But this doesn't exist right? We would need something similar to
v4l2_i2c_new_subdev_board() but for platform devices.
>
>> Following your approach I could get something like this:
>>
>> hdmi_system@YYYY {
>> compatible = "snps,dw-hdmi-rx-wrapper";
> This would need to refer to some hardware block, we should avoid virtual
> device nodes in DT.
By wrapper I mean the SoC, i.e. the whole integration of the
controller + phy + EDID + 5v + ...
>
>> reg = <0xYYYY 0xZZZZ>;
>> interrupts = <3>;
>> #address-cells = <1>;
>> #size-cells = <1>;
> You would need also an (empty) 'ranges' property here.
Yes, I already added it here locally.
>
>> hdmi_controller@0 {
>> compatible = "snps,dw-hdmi-rx-controller";
>> reg = <0x0 0x10000>;
>> interrupts = <1>;
>> edid-phandle = <&hdmi_system>;
>> clocks = <&refclk>;
>> clock-names = "ref-clk";
>> #address-cells = <1>;
>> #size-cells = <0>;
>>
>> hdmi_phy@f3 {
>> compatible = "snps,dw-hdmi-phy-e405";
>> reg = <0xf3>;
>> clocks = <&cfgclk>;
>> clock-names = "cfg-clk";
>> }
>> }
>> };
>>
>> And then snps,dw-hdmi-rx-wrapper would call of_platform_populate
>> for controller which would instead call of_platform_populate for
>> phy. Is this possible, and maintainable? Isn't the controller
>> driver get auto loaded because of the compatible string match?
> As I mentioned above, auto loading should work if you have instance
> of MODULE_DEVICE_TABLE() defined in the module, but the module might
> not be available immediately after creating devices with
> of_platform_populate(). You may want to have a look at the v4l2-async
> API (drivers/media/v4l2-core/v4l2-async.c). It allows one driver
> to register a notifier for its sub-devices. And the parent driver
> can complete initialization when it gets all its v4l2 subdevs
> registered.
>
> But I'm not sure about calls from the PHY back to the RX controller,
> possibly v4l2_set_subdev_hostdata()/v4l2_get_subdev_hostdata() could
> be used for passing the ops.
Ok, I will move to the async API then.
>
>> And one more thing: The reg address of the hdmi_controller: Isn't
>> this relative to the parent node? I mean isn't this going to be
>> 0xYYYY + 0x0? Because I don't want that :/
> Address space of child nodes doesn't need to be relative to 'reg' range
> of parent node, these can be entirely distinct address ranges. See for
> example I2C bus controllers, the I2C addresses of slave devices are not
> much related to the memory mapped IO registers region of the bus controller.
Yes, I already managed to get it working :)
Best regards,
Jose Miguel Abreu
>
> --
> Regards,
> Sylwester
