On Tue, Apr 09, 2024 at 01:04:12PM +0200, Pavel Machek wrote:
> Hi!
>
> > > This is driver for ANX7688 USB-C HDMI, with flashing and debugging
> > > features removed. ANX7688 is rather criticial piece on PinePhone,
> > > there's no display and no battery charging without it.
> > >
> > > There's likely more work to be done here, but having basic support
> > > in mainline is needed to be able to work on the other stuff
> > > (networking, cameras, power management).
> > >
> > > Signed-off-by: Ondrej Jirman <megi@xxxxxx>
> > > Co-developed-by: Martijn Braam <martijn@xxxxxxxxx>
> > > Co-developed-by: Samuel Holland <samuel@xxxxxxxxxxxx>
> > > Signed-off-by: Pavel Machek <pavel@xxxxxx>
> >
> > Just couple of quick comments below - I did not have time to go over
> > this very thoroughly, but I think you need to make a new version in
> > any case because of comments in 1/2.
>
[skipped]
>
> > > +static int anx7688_connect(struct anx7688 *anx7688)
> > > +{
> > > + struct typec_partner_desc desc = {};
> > > + int ret, i;
> > > + u8 fw[2];
> > > + const u8 dp_snk_identity[16] = {
> > > + 0x00, 0x00, 0x00, 0xec, /* id header */
> > > + 0x00, 0x00, 0x00, 0x00, /* cert stat */
> > > + 0x00, 0x00, 0x00, 0x00, /* product type */
> > > + 0x39, 0x00, 0x00, 0x51 /* alt mode adapter */
> > > + };
> > > + const u8 svid[4] = {
> > > + 0x00, 0x00, 0x01, 0xff,
> > > + };
> >
> > Why not get those from DT?
>
> Are you sure it belongs to the DT (and that DT people will agree)?
>From Documentation/devicetree/bindings/connector/usb-connector.yaml:
altmodes {
displayport {
svid = /bits/ 16 <0xff01>;
vdo = <0x00001c46>;
};
};
BTW, I don't see the VDO for the DP altmode in your code. Maybe I missed
it at a quick glance.
>
> > > + u32 caps[8];
> > > +
--
With best wishes
Dmitry
