On 05/20/2015 09:54 AM, Jon Hunter wrote:
On 20/05/15 16:40, Thierry Reding wrote:
* PGP Signed by an unknown key
On Wed, May 20, 2015 at 02:46:07PM +0100, Jon Hunter wrote:
On 19/05/15 15:46, Thierry Reding wrote:
Old Signed by an unknown key
On Mon, May 18, 2015 at 04:33:49PM +0100, Jon Hunter wrote:
Background:
==========
On tegra124 and tegra132 devices the pads used by the Display Port Auxiliary
(DPAUX) channel are multiplexed such that they can also be used by one of the
internal i2c controllers. Note that this is different from i2c-over-AUX
supported by the DPAUX controller. The register that configures these pads is
part of the DPAUX controllers register set and so requires the clock for the
DPAUX controller to be enabled to access the register as well as keeping the
SOR (serial output resource) power domain enabled.
Currently, there is no pinctrl device for these pads and so cannot be easily
mapped to function as an i2c interface. Furthermore, when using the pads for
the DPAUX channel, the DPAUX driver (drivers/gpu/drm/tegra/dpaux.c) directly
writes the to appropriate register to setup the pads.
There are some products based upon the tegra132 that use these pads for an
internal i2c controller and hence we want to support this configuration in the
kernel.
Good timing, I was going to (reluctantly) add this to my long TODO list.
I generally like the proposal.
Ok, great.
Proposal:
========
Add a DPAUX MFD device that consists of a DPAUX controller, for the Display
Port Auxiliary related functionality and a DPAUX pad controller, for handling
the pinctrl for the DPAUX pads. Both the DPAUX controller and DPAUX pad
controller need to access the DPAUX register set and therefore, by making the
MFD compatible with "simple-mfd" and "syscon", a regmap for the DPAUX registers
will be created to synchronise register accesses made by the drivers.
Can we not do without an MFD here? Not only would it break DT ABI, but
it's also way more complicated than it needs to be in my opinion, we're
only sharing a single register (or perhaps even two) after all. Keeping
everything in a single DT node would also make the binding less awkward
because the power domain doesn't apply to the pad controller part of
DPAUX.
Can't the dpaux driver simply register the pinmux controller itself?
Do you think something that looks like the below?
+Example (tegra124 DPAUX):
+
+/ {
+ ...
+
+ host1x {
+ compatible = "nvidia,tegra124-host1x", "simple-bus";
+ ...
+
+ dpaux: dpaux@0,545c0000 {
+ compatible = "nvidia,tegra124-dpaux",
+ reg = <0x0 0x545c0000 0x0 0x40000>;
+ interrupts = <GIC_SPI 159 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&tegra_car TEGRA124_CLK_DPAUX>,
+ <&tegra_car TEGRA124_CLK_PLL_DP>;
+ clock-names = "dpaux", "parent";
+ resets = <&tegra_car 181>;
+ reset-names = "dpaux";
+ pinctrl-0 = <&dpaux_state>;
+ pinctrl-names = "default";
+ status = "disabled";
+
+ dpaux_padctl@0,545c0124 {
+ compatible = "nvidia,tegra124-dpaux-padctl";
+
+ dpaux_state: dpaux_state0 {
+ dpaux {
+ nvidia,function = "dpaux";
+ };
+ };
+
+ i2c_state: i2c_state0 {
+ i2c {
+ nvidia,function = "i2c";
+ };
+ };
+ };
Why even have this subnode? Couldn't we simply have this:
host1x@... {
...
dpaux@... {
compatible = "nvidia,tegra124-dpaux";
...
pinctrl-0 = <&dpaux_aux_state>;
pinctrl-1 = <&dpaux_i2c_state>;
pinctrl-names = "aux", "i2c";
...
dpaux_aux_state: pinmux-aux {
...
};
dpaux_i2c_state: pinmux-i2c {
...
};
};
};
?
We might need to add in indices to tell apart DPAUX and DPAUX1, though
perhaps we could refer to these states by path instead of phandle to
avoid that. Anyway, I don't see any particular reason why a subnode
would be necessary.
My thinking was that we would have a pinctrl driver for dpaux in
drivers/pinctrl/pinctrl-tegra-dpaux.c and therefore, I had assumed that
we would need a sub-node and compatible string to probe the device.
Are you sugguesting that the pinctrl driver for dpaux lives in
drivers/gpu/drm/tegra/dpaux.c?
Sorry if I am misunderstanding something here.
I think a single DT node for the single HW block makes sense. IIUC, that
would most correctly reflect how the HW is actually structured.
I don't see any conceptual reason why the driver that binds to that node
can't register as both a pinctrl driver plus anything else it needs to.
For example, there are plenty of Linux drivers that register as both
GPIO and pinctrl drivers already. If having the same "struct device"
register with multiple subsystems doesn't work out (IIRC some subsystems
attempt to own the struct device's one driver_data field), then the
top-level driver can internally create whatever child devices it needs
to do its job. Using MFD to do that feels like overkill in this
situation since those child devices are unlikely to ever show up with
some different parent device or register offset. Either way, the choice
of whether to use MFD or not shouldn't affect the DT binding in any way.
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
