Received-SPF: pass (google.com: domain of linux-kernel+bounces-135586-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Date: Mon, 8 Apr 2024 17:17:41 +0200
From: =?utf-8?Q?Ond=C5=99ej?= Jirman <megi@xff.cz>
To: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Cc: Pavel Machek <pavel@ucw.cz>, phone-devel@vger.kernel.org, 
	kernel list <linux-kernel@vger.kernel.org>, fiona.klute@gmx.de, martijn@brixit.nl, samuel@sholland.org, 
	heikki.krogerus@linux.intel.com, gregkh@linuxfoundation.org, linux-usb@vger.kernel.org, 
	robh+dt@kernel.org, krzysztof.kozlowski+dt@linaro.org, devicetree@vger.kernel.org
Subject: Re: [PATCHv3 1/2] dt-bindings: usb: typec: anx7688: start a binding
 document
Message-ID: <vbo7bacecuagu4qzrr6tsdh4qlejrv7ia67yylf6ay4u7qnwge@kqj27bun2m7d>
Mail-Followup-To: =?utf-8?Q?Ond=C5=99ej?= Jirman <megi@xff.cz>, 
	Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>, Pavel Machek <pavel@ucw.cz>, phone-devel@vger.kernel.org, 
	kernel list <linux-kernel@vger.kernel.org>, fiona.klute@gmx.de, martijn@brixit.nl, samuel@sholland.org, 
	heikki.krogerus@linux.intel.com, gregkh@linuxfoundation.org, linux-usb@vger.kernel.org, 
	robh+dt@kernel.org, krzysztof.kozlowski+dt@linaro.org, devicetree@vger.kernel.org
References: <ZhPMHdt6r/4D99Zg@duo.ucw.cz>
 <ab9affc8-de68-4ec9-bdfc-02131191bc3a@linaro.org>
 <ZhPTTxI4oTF3pgrk@duo.ucw.cz>
 <e7841ad2-fa3d-442d-804d-51f12e05c234@linaro.org>
 <e6vvuttix5k5fioy7q44ick5wj6u5gleh7mht36s4zjjcym7vy@bziejyohtc4b>
 <7976e254-ed1e-406d-870b-1ecdc4b1e23c@linaro.org>
 <uoo7xltbfx7u6iai7urj3wez7cwotokxt6lwjhff57xbljusqn@fr2xejnrlak7>
 <1502383c-9caf-4362-8bd6-ed719a304f08@linaro.org>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <1502383c-9caf-4362-8bd6-ed719a304f08@linaro.org>

On Mon, Apr 08, 2024 at 03:27:00PM GMT, Krzysztof Kozlowski wrote:
> On 08/04/2024 14:48, Ondřej Jirman wrote:
> > Yeah, I understand where the confusion is. The driver is not for anx7688 chip
> > really. The driver is named anx7688, but that's mostly a historical accident at
> > this point.
> > 
> > I guess there can be a driver for anx7688 chip that can directly use the chip's
> > resources from the host by directly manipulating its registers and implementing
> > type-c functionality via eg. Linux's TCPM or TCPCI stack, etc. (eg. like
> > fusb302 driver, or various tcpci subdrivers).
> > 
> > But in this case the chip is driven by an optional on-chip microcontroller's
> > firmware and *this driver* is specifically for *the Type-C port on Pinephone*
> 
> We do not talk here about the driver, but bindings, so hardware.

Got it. Bindings should be the same regardless of what driver would be used,
whether this OCM based one, or some future one based on the above mentioned
TCPCI in-kernel implementation. Hardware is the same in both cases.

Just trying to imagine how to actually solve the issues...

Basic thing with the I2C regulator thing is that needs to be enabled as long
as anx7688 needs to communicate over I2C. Other user of this power rail is
touchscreen controller for its normal power supply, and it needs to be able
to disable it during system suspend.

Now for things to not fail during suspend/resume based on PM callbacks
invocation order, anx7688 driver needs to enable this regulator too, as long
as it needs it.

I can put bus-supply to I2C controller node, and read it from the ANX7688 driver
I guess, by going up a DT node. Whether that's going to be acceptable, I don't
know. 


VCONN regulator I don't know where else to put either. It doesn't seem to belong
anywhere. It's not something directly connected to Type-C connector, so
not part of connector bindings, and there's nothing else I can see, other
than anx7688 device which needs it for core functionality.

ANX7688 chip desing doesn't have integrated VCONN mosfet switches so it always
needs external supply + switches that are controlled by the chip itself. There's
no sensible design where someone would not want this and the driver needs
to get this regulator reference from somewhere. The switches are sort of an
extension of the chip.

kind regards,
	o.


> > and serves as an integration driver for quite a bunch of things that need to
> > work together on Pinephone for all of the Type-C port's features to operate
> > reasonably well (and one of those is some communication with anx7688 firmware
> > that we use, and enabling power to this chip and other things as appropriate,
> > based on the communication from the firmware).
> 
> That's still looking like putting board design into particular device
> binding.
> 
> > 
> > It handles the specific needs of the Pinephone's Type-C implementation, all of
> > its quirks (of which there are many over several HW revisions) that can't be
> > handled by the particular implementation of on-chip microcontroller firmware
> > directly and need host side interaction.
> > 
> > In an ideal world, many of the things this driver handles would be handled by
> > embedded microcontroller on the board (like it is with some RK3399 based Google
> > devices), but Pinephone has no such thing and this glue needs to be implemented
> > somewhere in the kernel.
> 
> You might need multiple schemas, because this is for anx7688, not for
> Pinephone type-c implementation.
> 
> However I still do not see yet a limitation of DTS requiring stuffing
> some other properties into anx7688 or creating some other, virtual entity.
> 
> 
> Best regards,
> Krzysztof
>