Re: [PATCH] usb: typec: anx7688: Add driver for ANX7688 USB-C HDMI bridge

[Greg KH]

On Thu, Feb 01, 2024 at 11:41:56AM +0100, Pavel Machek wrote:
> +#define DEBUG

Please don't.  This is dynamic, use the dynamic debugging and set it
from userspace if you want to debug the driver.

> +
> +#include <linux/debugfs.h>
> +#include <linux/delay.h>
> +#include <linux/extcon-provider.h>
> +#include <linux/firmware.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/i2c.h>
> +#include <linux/interrupt.h>
> +#include <linux/irqreturn.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of_irq.h>
> +#include <linux/power_supply.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/usb/pd.h>
> +#include <linux/usb/role.h>
> +#include <linux/usb/typec.h>
> +
> +/* firmware regs */
> +
> +#define ANX7688_REG_VBUS_OFF_DELAY_TIME 0x22
> +#define ANX7688_REG_FEATURE_CTRL        0x27
> +#define ANX7688_REG_EEPROM_LOAD_STATUS1 0x11
> +#define ANX7688_REG_EEPROM_LOAD_STATUS0 0x12
> +#define ANX7688_REG_FW_VERSION1         0x15
> +#define ANX7688_REG_FW_VERSION0         0x16
> +
> +#define ANX7688_EEPROM_FW_LOADED	0x01

Mix of tabs and spaces, please just use tabs.

> +static const char * const anx7688_supply_names[] = {
> +        "avdd33",
> +        "avdd18",
> +        "dvdd18",
> +        "avdd10",
> +        "dvdd10",
> +	"i2c",
> +        "hdmi_vt",
> +
> +        "vconn", // power for VCONN1/VCONN2 switches
> +        "vbus", // vbus power
> +};

Again, tabs vs. spaces, please use checkpatch.

> +#define ANX7688_NUM_SUPPLIES ARRAY_SIZE(anx7688_supply_names)
> +#define ANX7688_NUM_ALWAYS_ON_SUPPLIES (ANX7688_NUM_SUPPLIES - 1)
> +
> +#define ANX7688_I2C_INDEX (ANX7688_NUM_SUPPLIES - 4)
> +#define ANX7688_VCONN_INDEX (ANX7688_NUM_SUPPLIES - 2)
> +#define ANX7688_VBUS_INDEX (ANX7688_NUM_SUPPLIES - 1)
> +
> +enum {
> +	ANX7688_F_POWERED,
> +	ANX7688_F_CONNECTED,
> +	ANX7688_F_FW_FAILED,
> +	ANX7688_F_PWRSUPPLY_CHANGE,
> +	ANX7688_F_CURRENT_UPDATE,
> +};
> +
> +struct anx7688 {
> +        struct device *dev;
> +        struct i2c_client *client;
> +        struct i2c_client *client_tcpc;
> +        struct regulator_bulk_data supplies[ANX7688_NUM_SUPPLIES];
> +	struct power_supply *vbus_in_supply;
> +	struct notifier_block vbus_in_nb;
> +	int input_current_limit; // mA
> +        struct gpio_desc *gpio_enable;
> +        struct gpio_desc *gpio_reset;
> +        struct gpio_desc *gpio_cabledet;

I'm stopping here, again, tabs, you know this :(

greg k-h