From: Hans de Goede <hdegoede@redhat.com>
To: Andy Shevchenko <andy.shevchenko@gmail.com>
Cc: "Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>,
"Andy Shevchenko" <andy@kernel.org>,
platform-driver-x86@vger.kernel.org
Subject: Re: [PATCH v2] platform/x86: Add new MeeGoPad ANX7428 Type-C Cross Switch driver
Date: Wed, 15 May 2024 15:15:00 +0200 [thread overview]
Message-ID: <a2074a60-1d95-4366-9868-9bda4503f9d4@redhat.com> (raw)
In-Reply-To: <CAHp75Vdb3_yHmS1A0cixKGRXHu_aUg0a7+cXLvtQFT-DumDQgA@mail.gmail.com>
Hi,
On 5/15/24 6:53 AM, Andy Shevchenko wrote:
> On Tue, May 14, 2024 at 9:03 PM Hans de Goede <hdegoede@redhat.com> wrote:
>>
>> Some MeeGoPad top-set boxes have an ANX7428 Type-C Switch for USB3.1 Gen 1
>> and DisplayPort over Type-C alternate mode support.
>>
>> The ANX7428 has a microcontroller which takes care of the PD negotiation
>> and automatically sets the builtin Crosspoint Switch to send the right
>> signal to the 4 highspeed pairs of the Type-C connector. It also takes
>> care of HPD and AUX channel routing for DP alternate mode.
>>
>> IOW the ANX7428 operates fully autonomous and to the x5-Z8350 SoC
>> things look like there simple is a USB-3 Type-A connector and a
>> separate DisplayPort connector. Except that the BIOS does not
>> power on the ANX7428 at boot (meh).
>>
>> Add a driver to power on the ANX7428. This driver is added under
>> drivers/platform/x86 rather than under drivers/usb/typec for 2 reasons:
>>
>> 1. This driver is specifically written to work with how the ANX7428 is
>> described in the ACPI tables of the MeeGoPad x86 (Cherry Trail) devices.
>>
>> 2. This driver only powers on the ANX7428 and does not do anything wrt
>> its Type-C functionality. It should be possible to tell the controller
>> which data- and/or power-role to negotiate and to swap the role(s) after
>> negotiation but the MeeGoPad top-set boxes always draw their power from
>> a separate power-connector and they only support USB host-mode. So this
>> functionality is unnecessary and due to lack of documentation this is
>> tricky to support.
>
> ...
>
>> + * DisplayPort over Type-C alternate mode support.
>> + *
>> + * The ANX7428 has a microcontroller which takes care of the PD
>> + * negotiation and automatically sets the builtin Crosspoint Switch
>> + * to send the right signal to the 4 highspeed pairs of the Type-C
>> + * connector. It also takes care of HPD and AUX channel routing for
>> + * DP alternate mode.
>> + *
>> + * IOW the ANX7428 operates fully autonomous and to the x5-Z8350 SoC
>> + * things look like there simple is a USB-3 Type-A connector and a
>
> simply
>
>> + * separate DisplayPort connector. Except that the BIOS does not
>> + * power on the ANX7428 at boot. This driver takes care of powering
>> + * on the ANX7428.
>> + *
>> + * It should be possible to tell the micro-controller which data- and/or
>> + * power-role to negotiate and to swap the role(s) after negotiation
>> + * but the MeeGoPad top-set boxes always draw their power from a separate
>> + * power-connector and they only support USB host-mode. So this functionality
>> + * is unnecessary and due to lack of documentation this is tricky to support.
>> + *
>> + * For a more complete ANX7428 driver see drivers/usb/misc/anx7418/ of
>> + * the LineageOS kernel for the LG G5 (International) aka the LG H850:
>> + * https://github.com/LineageOS/android_kernel_lge_msm8996/
>
> ...
>
>> +#include <linux/acpi.h>
>> +#include <linux/bits.h>
>> +#include <linux/delay.h>
>
> + dev_printk.h
>
>> +#include <linux/dmi.h>
>
> + err.h
>
>> +#include <linux/gpio/consumer.h>
>> +#include <linux/i2c.h>
>> +#include <linux/iopoll.h>
>> +#include <linux/module.h>
>
> + types.h
>
> ...
>
>> +static struct i2c_driver anx7428_driver = {
>> + .driver = {
>> + .name = "meegopad_anx7428",
>> + .acpi_match_table = anx7428_acpi_match,
>> + },
>> + .probe = anx7428_probe,
>> +};
>
>> +
>
> Unneeded blank line.
>
>> +module_i2c_driver(anx7428_driver);
>
> ...
>
> You can fold the above into the current one, no need to resend.
Thank you for the review.
I've squashed fixes for all of these into the existing commit.
Regards,
Hans
prev parent reply other threads:[~2024-05-15 13:15 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-14 18:03 [PATCH v2] platform/x86: Add new MeeGoPad ANX7428 Type-C Cross Switch driver Hans de Goede
2024-05-14 18:14 ` Hans de Goede
2024-05-15 4:53 ` Andy Shevchenko
2024-05-15 13:15 ` Hans de Goede [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=a2074a60-1d95-4366-9868-9bda4503f9d4@redhat.com \
--to=hdegoede@redhat.com \
--cc=andy.shevchenko@gmail.com \
--cc=andy@kernel.org \
--cc=ilpo.jarvinen@linux.intel.com \
--cc=platform-driver-x86@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).