Re: [PATCHv3 3/3] drm: bridge: anx78xx: Add anx78xx driver support by analogix.

From: Dan Carpenter
Date: Tue Sep 22 2015 - 15:44:37 EST


On Thu, Sep 10, 2015 at 06:35:52PM +0200, Enric Balletbo i Serra wrote:
> diff --git a/drivers/gpu/drm/bridge/anx78xx/anx78xx.h b/drivers/gpu/drm/bridge/anx78xx/anx78xx.h
> new file mode 100644
> index 0000000..4f6dd1d
> --- /dev/null
> +++ b/drivers/gpu/drm/bridge/anx78xx/anx78xx.h
> @@ -0,0 +1,44 @@
> +/*
> + * Copyright(c) 2015, Analogix Semiconductor. All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 and
> + * only version 2 as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + */
> +
> +#ifndef __ANX78xx_H
> +#define __ANX78xx_H
> +
> +#include <linux/i2c.h>
> +#include <linux/mutex.h>
> +#include <linux/slab.h>
> +#include <linux/workqueue.h>
> +#include <linux/gpio/consumer.h>
> +
> +#define AUX_ERR 1
> +#define AUX_OK 0

Get rid of these. They aren't used much and we could easily use normal
error codes instead.

> +
> +struct anx78xx_platform_data {
> + struct gpio_desc *gpiod_pd;
> + struct gpio_desc *gpiod_reset;
> + spinlock_t lock;
> +};
> +
> +struct anx78xx {
> + struct i2c_client *client;
> + struct anx78xx_platform_data *pdata;
> + struct delayed_work work;
> + struct workqueue_struct *workqueue;
> + struct mutex lock;
> +};
> +
> +void anx78xx_poweron(struct anx78xx *data);
> +void anx78xx_poweroff(struct anx78xx *data);
> +
> +#endif
> diff --git a/drivers/gpu/drm/bridge/anx78xx/anx78xx_main.c b/drivers/gpu/drm/bridge/anx78xx/anx78xx_main.c
> new file mode 100644
> index 0000000..b92d2bc
> --- /dev/null
> +++ b/drivers/gpu/drm/bridge/anx78xx/anx78xx_main.c
> @@ -0,0 +1,241 @@
> +/*
> + * Copyright(c) 2015, Analogix Semiconductor. All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 and
> + * only version 2 as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + */
> +#include <linux/delay.h>
> +#include <linux/interrupt.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/types.h>
> +#include <linux/err.h>
> +#include <linux/async.h>
> +#include <linux/of_gpio.h>
> +#include <linux/of_platform.h>
> +#include <linux/delay.h>
> +
> +#include "anx78xx.h"
> +#include "slimport_tx_drv.h"
> +
> +void anx78xx_poweron(struct anx78xx *anx78xx)
> +{
> + struct device *dev = &anx78xx->client->dev;
> + struct anx78xx_platform_data *pdata = anx78xx->pdata;
> +
> + gpiod_set_value_cansleep(pdata->gpiod_reset, 0);
> + usleep_range(1000, 2000);
> +
> + gpiod_set_value_cansleep(pdata->gpiod_pd, 0);
> + usleep_range(1000, 2000);
> +
> + gpiod_set_value_cansleep(pdata->gpiod_reset, 1);
> +
> + dev_dbg(dev, "power on\n");

Remove these debug printks. Use ftrace instead.

> +}
> +
> +void anx78xx_poweroff(struct anx78xx *anx78xx)
> +{
> + struct device *dev = &anx78xx->client->dev;
> + struct anx78xx_platform_data *pdata = anx78xx->pdata;
> +
> + gpiod_set_value_cansleep(pdata->gpiod_reset, 0);
> + usleep_range(1000, 2000);
> +
> + gpiod_set_value_cansleep(pdata->gpiod_pd, 1);
> + usleep_range(1000, 2000);
> +
> + dev_dbg(dev, "power down\n");

Delete.

> +}
> +
> +static int anx78xx_init_gpio(struct anx78xx *anx78xx)
> +{
> + struct device *dev = &anx78xx->client->dev;
> + struct anx78xx_platform_data *pdata = anx78xx->pdata;
> + int ret;
> +
> + /* gpio for chip power down */
> + pdata->gpiod_pd = devm_gpiod_get(dev, "pd", GPIOD_OUT_HIGH);
> + if (IS_ERR(pdata->gpiod_pd)) {
> + dev_err(dev, "unable to claim pd gpio\n");
> + ret = PTR_ERR(pdata->gpiod_pd);
> + return ret;


The ret variable isn't necessary in this function.

> + }
> +
> + /* gpio for chip reset */
> + pdata->gpiod_reset = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
> + if (IS_ERR(pdata->gpiod_reset)) {
> + dev_err(dev, "unable to claim reset gpio\n");
> + ret = PTR_ERR(pdata->gpiod_reset);
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static int anx78xx_system_init(struct anx78xx *anx78xx)
> +{
> + struct device *dev = &anx78xx->client->dev;
> + int ret;
> +
> + ret = sp_chip_detect(anx78xx);

Make sp_chip_detect() use normal error codes.

> + if (ret == 0) {
> + anx78xx_poweroff(anx78xx);
> + dev_err(dev, "failed to detect anx78xx\n");
> + return -ENODEV;
> + }
> +
> + sp_tx_variable_init();
> + return 0;
> +}
> +
> +static void anx78xx_work_func(struct work_struct *work)
> +{
> + struct anx78xx *anx78xx = container_of(work, struct anx78xx,
> + work.work);
> + int workqueu_timer = 0;
> +
> + if (sp_tx_cur_states() >= STATE_PLAY_BACK)
> + workqueu_timer = 500;
> + else
> + workqueu_timer = 100;
> + mutex_lock(&anx78xx->lock);
> + sp_main_process(anx78xx);
> + mutex_unlock(&anx78xx->lock);
> + queue_delayed_work(anx78xx->workqueue, &anx78xx->work,
> + msecs_to_jiffies(workqueu_timer));
> +}
> +
> +static int anx78xx_i2c_probe(struct i2c_client *client,
> + const struct i2c_device_id *id)
> +{
> + struct anx78xx *anx78xx;
> + int ret;
> +
> + if (!i2c_check_functionality(client->adapter,
> + I2C_FUNC_SMBUS_I2C_BLOCK)) {

Use checkpatch.pl --strict.

if (!i2c_check_functionality(client->adapter,
I2C_FUNC_SMBUS_I2C_BLOCK)) {

> + dev_err(&client->dev, "i2c bus does not support the device\n");
> + return -ENODEV;
> + }
> +
> + anx78xx = devm_kzalloc(&client->dev,
> + sizeof(struct anx78xx),
> + GFP_KERNEL);

Better style is:

anx78xx = devm_kzalloc(&client->dev, sizeof(*anx78xx), GFP_KERNEL);

> + if (!anx78xx)
> + return -ENOMEM;
> +
> + anx78xx->pdata = devm_kzalloc(&client->dev,
> + sizeof(struct anx78xx_platform_data),
> + GFP_KERNEL);
> + if (!anx78xx->pdata)
> + return -ENOMEM;
> +
> + anx78xx->client = client;
> +
> + i2c_set_clientdata(client, anx78xx);
> +
> + mutex_init(&anx78xx->lock);
> +
> + ret = anx78xx_init_gpio(anx78xx);
> + if (ret) {
> + dev_err(&client->dev, "failed to initialize gpios\n");
> + return ret;
> + }
> +
> + INIT_DELAYED_WORK(&anx78xx->work, anx78xx_work_func);
> +
> + anx78xx->workqueue = create_singlethread_workqueue("anx78xx_work");
> + if (anx78xx->workqueue == NULL) {
> + dev_err(&client->dev, "failed to create work queue\n");
> + return -ENOMEM;
> + }
> +
> + ret = anx78xx_system_init(anx78xx);
> + if (ret) {
> + dev_err(&client->dev, "failed to initialize anx78xx\n");
> + goto cleanup;
> + }
> +
> + /* enable driver */
> + queue_delayed_work(anx78xx->workqueue, &anx78xx->work, 0);
> +
> + return 0;
> +
> +cleanup:
> + destroy_workqueue(anx78xx->workqueue);
> + return ret;
> +}
> +
> +static int anx78xx_i2c_remove(struct i2c_client *client)
> +{
> + struct anx78xx *anx78xx = i2c_get_clientdata(client);
> +
> + destroy_workqueue(anx78xx->workqueue);
> +
> + return 0;
> +}
> +
> +static int anx78xx_i2c_suspend(struct device *dev)
> +{
> + struct i2c_client *client = container_of(dev, struct i2c_client, dev);
> + struct anx78xx *anx78xx = i2c_get_clientdata(client);
> +
> + cancel_delayed_work_sync(&anx78xx->work);
> + flush_workqueue(anx78xx->workqueue);
> + anx78xx_poweroff(anx78xx);
> + sp_tx_clean_state_machine();
> +
> + return 0;
> +}
> +
> +static int anx78xx_i2c_resume(struct device *dev)
> +{
> + struct i2c_client *client = container_of(dev, struct i2c_client, dev);
> + struct anx78xx *anx78xx = i2c_get_clientdata(client);
> +
> + queue_delayed_work(anx78xx->workqueue, &anx78xx->work, 0);
> +
> + return 0;
> +}
> +
> +static SIMPLE_DEV_PM_OPS(anx78xx_i2c_pm_ops,
> + anx78xx_i2c_suspend, anx78xx_i2c_resume);
> +
> +static const struct i2c_device_id anx78xx_id[] = {
> + {"anx7814", 0},
> + { /* sentinel */ }
> +};
> +
> +MODULE_DEVICE_TABLE(i2c, anx78xx_id);
> +
> +static const struct of_device_id anx78xx_match_table[] = {
> + {.compatible = "analogix,anx7814",},
> + { /* sentinel */ },
> +};
> +
> +MODULE_DEVICE_TABLE(of, anx78xx_match_table);
> +
> +static struct i2c_driver anx78xx_driver = {
> + .driver = {
> + .name = "anx7814",
> + .pm = &anx78xx_i2c_pm_ops,
> + .of_match_table = of_match_ptr(anx78xx_match_table),
> + },
> + .probe = anx78xx_i2c_probe,
> + .remove = anx78xx_i2c_remove,
> + .id_table = anx78xx_id,
> +};
> +
> +module_i2c_driver(anx78xx_driver);
> +
> +MODULE_DESCRIPTION("Slimport transmitter ANX78XX driver");
> +MODULE_AUTHOR("Junhua Xia <jxia@xxxxxxxxxxxxxxxx>");
> +MODULE_LICENSE("GPL v2");
> +MODULE_VERSION("1.1");
> diff --git a/drivers/gpu/drm/bridge/anx78xx/slimport_tx_drv.c b/drivers/gpu/drm/bridge/anx78xx/slimport_tx_drv.c
> new file mode 100644
> index 0000000..1be7f69
> --- /dev/null
> +++ b/drivers/gpu/drm/bridge/anx78xx/slimport_tx_drv.c
> @@ -0,0 +1,3198 @@
> +/*
> + * Copyright(c) 2015, Analogix Semiconductor. All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 and
> + * only version 2 as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + */
> +#include <linux/delay.h>
> +#include <linux/types.h>
> +
> +#include "anx78xx.h"
> +#include "slimport_tx_drv.h"
> +
> +#define XTAL_CLK_M10 pxtal_data[XTAL_27M].xtal_clk_m10
> +#define XTAL_CLK pxtal_data[XTAL_27M].xtal_clk
> +
> +struct slimport {
> + int block_en; /* HDCP control enable/ disable from AP */
> +
> + u8 tx_test_bw;
> + bool tx_test_lt;
> + bool tx_test_edid;
> +
> + u8 changed_bandwidth;
> +
> + u8 hdmi_dvi_status;
> + u8 need_clean_status;
> +
> + u8 ds_vid_stb_cntr;
> + u8 hdcp_fail_count;
> +
> + u8 edid_break;
> + u8 edid_checksum;
> + u8 edid_blocks[256];
> +
> + u8 read_edid_flag;
> +
> + u8 down_sample_en;
> +
> + struct packet_avi tx_packet_avi;
> + struct packet_spd tx_packet_spd;
> + struct packet_mpeg tx_packet_mpeg;
> + struct audio_info_frame tx_audioinfoframe;
> +
> + struct common_int common_int_status;
> + struct hdmi_rx_int hdmi_rx_int_status;
> +
> + enum sp_tx_state tx_system_state;
> + enum sp_tx_state tx_system_state_bak;
> + enum audio_output_status tx_ao_state;
> + enum video_output_status tx_vo_state;
> + enum sink_connection_status tx_sc_state;
> + enum sp_tx_lt_status tx_lt_state;
> + enum hdcp_status hcdp_state;
> +};
> +
> +static struct slimport sp;
> +
> +static const u16 chipid_list[] = {
> + 0x7818,
> + 0x7816,
> + 0x7814,
> + 0x7812,
> + 0x7810,
> + 0x7806,
> + 0x7802
> +};
> +
> +static void sp_hdmi_rx_new_vsi_int(struct anx78xx *anx78xx);
> +static u8 sp_hdcp_cap_check(struct anx78xx *anx78xx);
> +static void sp_tx_show_information(struct anx78xx *anx78xx);
> +static void sp_print_sys_state(struct anx78xx *anx78xx, u8 ss);
> +
> +static int sp_read_reg(struct anx78xx *anx78xx, u8 slave_addr,
> + u8 offset, u8 *buf)
> +{
> + u8 ret;

"ret" should be an int. It causes a signedness bug later.

> + struct i2c_client *client = anx78xx->client;
> +
> + client->addr = (slave_addr >> 1);
> +
> + ret = i2c_smbus_read_byte_data(client, offset);
> + if (ret < 0) {
> + dev_err(&client->dev, "failed to read i2c addr=%x\n",
> + slave_addr);
> + return ret;
> + }
> +
> + *buf = ret;
> +
> + return 0;
> +}
> +
> +static int sp_write_reg(struct anx78xx *anx78xx, u8 slave_addr,
> + u8 offset, u8 value)
> +{
> + int ret;
> + struct i2c_client *client = anx78xx->client;
> +
> + client->addr = (slave_addr >> 1);
> +
> + ret = i2c_smbus_write_byte_data(client, offset, value);
> + if (ret < 0)
> + dev_err(&client->dev, "failed to write i2c addr=%x\n",
> + slave_addr);
> +
> + return ret;
> +}
> +
> +static u8 sp_i2c_read_byte(struct anx78xx *anx78xx,
> + u8 dev, u8 offset)
> +{
> + u8 ret;
> +
> + sp_read_reg(anx78xx, dev, offset, &ret);
> + return ret;

Ugh... None of the callers check sp_read_reg() for failure...

> +}
> +
> +static void sp_reg_bit_ctl(struct anx78xx *anx78xx, u8 addr, u8 offset,
> + u8 data, bool enable)
> +{
> + u8 c;
> +
> + sp_read_reg(anx78xx, addr, offset, &c);
> + if (enable) {
> + if ((c & data) != data) {
> + c |= data;
> + sp_write_reg(anx78xx, addr, offset, c);
> + }
> + } else
> + if ((c & data) == data) {
> + c &= ~data;
> + sp_write_reg(anx78xx, addr, offset, c);
> + }

Put curly braces around the else statement for two style reasons.
1) If one side of an if else statement has curly braces then both get
them.
2) Multi-line indents get curly braces for readability.

> +}
> +
> +static inline void sp_write_reg_or(struct anx78xx *anx78xx, u8 address,
> + u8 offset, u8 mask)
> +{
> + sp_write_reg(anx78xx, address, offset,
> + sp_i2c_read_byte(anx78xx, address, offset) | mask);
> +}
> +
> +static inline void sp_write_reg_and(struct anx78xx *anx78xx, u8 address,
> + u8 offset, u8 mask)
> +{
> + sp_write_reg(anx78xx, address, offset,
> + sp_i2c_read_byte(anx78xx, address, offset) & mask);
> +}
> +
> +static inline void sp_write_reg_and_or(struct anx78xx *anx78xx, u8 address,
> + u8 offset, u8 and_mask, u8 or_mask)
> +{
> + sp_write_reg(anx78xx, address, offset,
> + (sp_i2c_read_byte(anx78xx, address, offset) & and_mask) | or_mask);
> +}
> +
> +static inline void sp_write_reg_or_and(struct anx78xx *anx78xx, u8 address,
> + u8 offset, u8 or_mask, u8 and_mask)
> +{
> + sp_write_reg(anx78xx, address, offset,
> + (sp_i2c_read_byte(anx78xx, address, offset) | or_mask) & and_mask);
> +}
> +
> +static inline void sp_tx_video_mute(struct anx78xx *anx78xx, bool enable)
> +{
> + sp_reg_bit_ctl(anx78xx, TX_P2, VID_CTRL1, VIDEO_MUTE, enable);
> +}
> +
> +static inline void hdmi_rx_mute_audio(struct anx78xx *anx78xx, bool enable)
> +{
> + sp_reg_bit_ctl(anx78xx, RX_P0, RX_MUTE_CTRL, AUD_MUTE, enable);
> +}
> +
> +static inline void hdmi_rx_mute_video(struct anx78xx *anx78xx, bool enable)
> +{
> + sp_reg_bit_ctl(anx78xx, RX_P0, RX_MUTE_CTRL, VID_MUTE, enable);
> +}
> +
> +static inline void sp_tx_addronly_set(struct anx78xx *anx78xx, bool enable)
> +{
> + sp_reg_bit_ctl(anx78xx, TX_P0, AUX_CTRL2, ADDR_ONLY_BIT, enable);
> +}
> +
> +static inline void sp_tx_set_link_bw(struct anx78xx *anx78xx, u8 bw)
> +{
> + sp_write_reg(anx78xx, TX_P0, SP_TX_LINK_BW_SET_REG, bw);
> +}
> +
> +static inline u8 sp_tx_get_link_bw(struct anx78xx *anx78xx)
> +{
> + return sp_i2c_read_byte(anx78xx, TX_P0, SP_TX_LINK_BW_SET_REG);
> +}
> +
> +static inline bool sp_tx_get_pll_lock_status(struct anx78xx *anx78xx)
> +{
> + u8 temp;
> +
> + temp = sp_i2c_read_byte(anx78xx, TX_P0, TX_DEBUG1);
> +
> + return (temp & DEBUG_PLL_LOCK) != 0 ? true : false;

Adding != 0 is just a double negative. It adds verbiage and extra words
but it doesn't make the code more clear.

> +}
> +
> +static inline void gen_m_clk_with_downspeading(struct anx78xx *anx78xx)
> +{
> + sp_write_reg_or(anx78xx, TX_P0, SP_TX_M_CALCU_CTRL, M_GEN_CLK_SEL);
> +}
> +
> +static inline void gen_m_clk_without_downspeading(struct anx78xx *anx78xx)
> +{
> + sp_write_reg_and(anx78xx, TX_P0, SP_TX_M_CALCU_CTRL, (~M_GEN_CLK_SEL));
> +}
> +
> +static inline void hdmi_rx_set_hpd(struct anx78xx *anx78xx, bool enable)
> +{
> + if (enable)
> + sp_write_reg_or(anx78xx, TX_P2, SP_TX_VID_CTRL3_REG, HPD_OUT);
> + else
> + sp_write_reg_and(anx78xx, TX_P2, SP_TX_VID_CTRL3_REG, ~HPD_OUT);
> +}
> +
> +static inline void hdmi_rx_set_termination(struct anx78xx *anx78xx,
> + bool enable)
> +{
> + if (enable)
> + sp_write_reg_and(anx78xx, RX_P0, HDMI_RX_TMDS_CTRL_REG7,
> + ~TERM_PD);
> + else
> + sp_write_reg_or(anx78xx, RX_P0, HDMI_RX_TMDS_CTRL_REG7,
> + TERM_PD);
> +}
> +
> +static inline void sp_tx_clean_hdcp_status(struct anx78xx *anx78xx)
> +{
> + struct device *dev = &anx78xx->client->dev;
> +
> + sp_write_reg(anx78xx, TX_P0, TX_HDCP_CTRL0, 0x03);
> + sp_write_reg_or(anx78xx, TX_P0, TX_HDCP_CTRL0, RE_AUTH);
> + usleep_range(2000, 4000);
> + dev_dbg(dev, "sp_tx_clean_hdcp_status\n");
> +}
> +
> +static inline void sp_tx_link_phy_initialization(struct anx78xx *anx78xx)
> +{
> + sp_write_reg(anx78xx, TX_P2, SP_TX_ANALOG_CTRL0, 0x02);
> + sp_write_reg(anx78xx, TX_P1, SP_TX_LT_CTRL_REG0, 0x01);
> + sp_write_reg(anx78xx, TX_P1, SP_TX_LT_CTRL_REG10, 0x00);
> + sp_write_reg(anx78xx, TX_P1, SP_TX_LT_CTRL_REG1, 0x03);
> + sp_write_reg(anx78xx, TX_P1, SP_TX_LT_CTRL_REG11, 0x00);
> + sp_write_reg(anx78xx, TX_P1, SP_TX_LT_CTRL_REG2, 0x07);
> + sp_write_reg(anx78xx, TX_P1, SP_TX_LT_CTRL_REG12, 0x00);
> + sp_write_reg(anx78xx, TX_P1, SP_TX_LT_CTRL_REG3, 0x7f);
> + sp_write_reg(anx78xx, TX_P1, SP_TX_LT_CTRL_REG13, 0x00);
> + sp_write_reg(anx78xx, TX_P1, SP_TX_LT_CTRL_REG4, 0x71);
> + sp_write_reg(anx78xx, TX_P1, SP_TX_LT_CTRL_REG14, 0x0c);
> + sp_write_reg(anx78xx, TX_P1, SP_TX_LT_CTRL_REG5, 0x6b);
> + sp_write_reg(anx78xx, TX_P1, SP_TX_LT_CTRL_REG15, 0x42);
> + sp_write_reg(anx78xx, TX_P1, SP_TX_LT_CTRL_REG6, 0x7f);
> + sp_write_reg(anx78xx, TX_P1, SP_TX_LT_CTRL_REG16, 0x1e);
> + sp_write_reg(anx78xx, TX_P1, SP_TX_LT_CTRL_REG7, 0x73);
> + sp_write_reg(anx78xx, TX_P1, SP_TX_LT_CTRL_REG17, 0x3e);
> + sp_write_reg(anx78xx, TX_P1, SP_TX_LT_CTRL_REG8, 0x7f);
> + sp_write_reg(anx78xx, TX_P1, SP_TX_LT_CTRL_REG18, 0x72);
> + sp_write_reg(anx78xx, TX_P1, SP_TX_LT_CTRL_REG9, 0x7F);
> + sp_write_reg(anx78xx, TX_P1, SP_TX_LT_CTRL_REG19, 0x7e);
> +}
> +
> +static inline void sp_tx_set_sys_state(struct anx78xx *anx78xx, u8 ss)
> +{
> + struct device *dev = &anx78xx->client->dev;
> +
> + dev_dbg(dev, "set: clean_status: %x,\n ", (u16)sp.need_clean_status);
> +
> + if ((sp.tx_system_state >= STATE_LINK_TRAINING)
> + && (ss < STATE_LINK_TRAINING))
> + sp_write_reg_or(anx78xx, TX_P0, SP_TX_ANALOG_PD_REG, CH0_PD);
> +
> + sp.tx_system_state_bak = sp.tx_system_state;
> + sp.tx_system_state = ss;
> + sp.need_clean_status = 1;
> + sp_print_sys_state(anx78xx, sp.tx_system_state);
> +}
> +
> +static inline void reg_hardware_reset(struct anx78xx *anx78xx)
> +{
> + sp_write_reg_or(anx78xx, TX_P2, SP_TX_RST_CTRL_REG, HW_RST);
> + sp_tx_clean_state_machine();
> + sp_tx_set_sys_state(anx78xx, STATE_SP_INITIALIZED);
> + msleep(500);
> +}
> +
> +static inline void write_dpcd_addr(struct anx78xx *anx78xx, u8 addrh,
> + u8 addrm, u8 addrl)
> +{
> + u8 temp;
> +
> + if (sp_i2c_read_byte(anx78xx, TX_P0, AUX_ADDR_7_0) != addrl)
> + sp_write_reg(anx78xx, TX_P0, AUX_ADDR_7_0, addrl);
> +
> + if (sp_i2c_read_byte(anx78xx, TX_P0, AUX_ADDR_15_8) != addrm)
> + sp_write_reg(anx78xx, TX_P0, AUX_ADDR_15_8, addrm);
> +
> + sp_read_reg(anx78xx, TX_P0, AUX_ADDR_19_16, &temp);
> +
> + if ((temp & 0x0F) != (addrh & 0x0F))
> + sp_write_reg(anx78xx, TX_P0, AUX_ADDR_19_16,
> + (temp & 0xF0) | addrh);
> +}
> +
> +static inline void goto_next_system_state(struct anx78xx *anx78xx)
> +{
> + struct device *dev = &anx78xx->client->dev;
> +
> + dev_dbg(dev, "next: clean_status: %x,\n ", (u16)sp.need_clean_status);
> +
> + sp.tx_system_state_bak = sp.tx_system_state;
> + sp.tx_system_state++;
> + sp_print_sys_state(anx78xx, sp.tx_system_state);
> +}
> +
> +static inline void redo_cur_system_state(struct anx78xx *anx78xx)
> +{
> + struct device *dev = &anx78xx->client->dev;
> +
> + dev_dbg(dev, "redo: clean_status: %x,\n ", (u16)sp.need_clean_status);
> +
> + sp.need_clean_status = 1;
> + sp.tx_system_state_bak = sp.tx_system_state;
> + sp_print_sys_state(anx78xx, sp.tx_system_state);
> +}
> +
> +static inline void system_state_change_with_case(struct anx78xx *anx78xx,
> + u8 status)
> +{
> + struct device *dev = &anx78xx->client->dev;
> +
> + if (sp.tx_system_state >= status) {

Flip this condition around and pull the code in one indent level.

if (status < sp.tx_system_state)
return;

> + dev_dbg(dev, "change_case: clean_status: %xm,\n ",

No extra space after the newline.

> + (u16)sp.need_clean_status);
> +
> + if ((sp.tx_system_state >= STATE_LINK_TRAINING)
> + && (status < STATE_LINK_TRAINING))

This should be aligned like this:

if (sp.tx_system_state >= STATE_LINK_TRAINING &&
status < STATE_LINK_TRAINING)

1) Removed extra parens.
2) Move the && to the first line
3) Changed the alignment

> + sp_write_reg_or(anx78xx, TX_P0, SP_TX_ANALOG_PD_REG,
> + CH0_PD);
> +
> + sp.need_clean_status = 1;
> + sp.tx_system_state_bak = sp.tx_system_state;
> + sp.tx_system_state = status;
> + sp_print_sys_state(anx78xx, sp.tx_system_state);
> + }
> +}
> +
> +static void sp_wait_aux_op_finish(struct anx78xx *anx78xx, u8 *err_flag)

Return an error. Don't use an err_flag pointer.

> +{
> + u8 cnt;
> + u8 c;
> + struct device *dev = &anx78xx->client->dev;
> +
> + *err_flag = 0;
> + cnt = 150;
> + while (sp_i2c_read_byte(anx78xx, TX_P0, AUX_CTRL2) & AUX_OP_EN) {
> + usleep_range(2000, 4000);
> + if ((cnt--) == 0) {

It's harmless but this will loop 151 times and not 150 as intended.
Putting parenthesis around a post-op doesn't change it to a pre-op.

> + dev_err(dev, "aux operate failed!\n");
> + *err_flag = 1;
> + break;
> + }
> + }
> +
> + sp_read_reg(anx78xx, TX_P0, SP_TX_AUX_STATUS, &c);
> + if (c & 0x0F) {
> + dev_err(dev, "wait aux operation status %.2x\n", (u16)c);
> + *err_flag = 1;
> + }
> +}
> +
> +static void sp_print_sys_state(struct anx78xx *anx78xx, u8 ss)
> +{
> + struct device *dev = &anx78xx->client->dev;
> +
> + switch (ss) {
> + case STATE_WAITTING_CABLE_PLUG:
> + dev_dbg(dev, "-STATE_WAITTING_CABLE_PLUG-\n");
> + break;
> + case STATE_SP_INITIALIZED:
> + dev_dbg(dev, "-STATE_SP_INITIALIZED-\n");
> + break;
> + case STATE_SINK_CONNECTION:
> + dev_dbg(dev, "-STATE_SINK_CONNECTION-\n");
> + break;
> + case STATE_PARSE_EDID:
> + dev_dbg(dev, "-STATE_PARSE_EDID-\n");
> + break;
> + case STATE_LINK_TRAINING:
> + dev_dbg(dev, "-STATE_LINK_TRAINING-\n");
> + break;
> + case STATE_VIDEO_OUTPUT:
> + dev_dbg(dev, "-STATE_VIDEO_OUTPUT-\n");
> + break;
> + case STATE_HDCP_AUTH:
> + dev_dbg(dev, "-STATE_HDCP_AUTH-\n");
> + break;
> + case STATE_AUDIO_OUTPUT:
> + dev_dbg(dev, "-STATE_AUDIO_OUTPUT-\n");
> + break;
> + case STATE_PLAY_BACK:
> + dev_dbg(dev, "-STATE_PLAY_BACK-\n");
> + break;
> + default:
> + dev_err(dev, "system state is error1\n");
> + break;
> + }
> +}
> +
> +static void sp_tx_rst_aux(struct anx78xx *anx78xx)
> +{
> + sp_write_reg_or(anx78xx, TX_P2, RST_CTRL2, AUX_RST);
> + sp_write_reg_and(anx78xx, TX_P2, RST_CTRL2, ~AUX_RST);
> +}
> +
> +static u8 sp_tx_aux_dpcdread_bytes(struct anx78xx *anx78xx, u8 addrh,
> + u8 addrm, u8 addrl, u8 ccount, u8 *pbuf)
> +{
> + u8 c, c1, i;
> + u8 bok;
> + struct device *dev = &anx78xx->client->dev;
> +
> + sp_write_reg(anx78xx, TX_P0, BUF_DATA_COUNT, 0x80);
> + c = ((ccount - 1) << 4) | 0x09;
> + sp_write_reg(anx78xx, TX_P0, AUX_CTRL, c);
> + write_dpcd_addr(anx78xx, addrh, addrm, addrl);
> + sp_write_reg_or(anx78xx, TX_P0, AUX_CTRL2, AUX_OP_EN);
> + usleep_range(2000, 4000);
> +
> + sp_wait_aux_op_finish(anx78xx, &bok);
> + if (bok == AUX_ERR) {
> + dev_err(dev, "aux read failed\n");
> + sp_read_reg(anx78xx, TX_P2, SP_TX_INT_STATUS1, &c);
> + sp_read_reg(anx78xx, TX_P0, TX_DEBUG1, &c1);
> + if (c1 & POLLING_EN) {
> + if (c & POLLING_ERR)
> + sp_tx_rst_aux(anx78xx);
> + } else
> + sp_tx_rst_aux(anx78xx);
> + return AUX_ERR;
> + }
> +
> + for (i = 0; i < ccount; i++) {
> + sp_read_reg(anx78xx, TX_P0, BUF_DATA_0 + i, &c);
> + *(pbuf + i) = c;
> + if (i >= MAX_BUF_CNT)
> + break;
> + }
> + return AUX_OK;
> +}
> +
> +static u8 sp_tx_aux_dpcdwrite_bytes(struct anx78xx *anx78xx, u8 addrh,
> + u8 addrm, u8 addrl, u8 ccount, u8 *pbuf)
> +{
> + u8 c, i, ret;
> +
> + c = ((ccount - 1) << 4) | 0x08;
> + sp_write_reg(anx78xx, TX_P0, AUX_CTRL, c);
> + write_dpcd_addr(anx78xx, addrh, addrm, addrl);
> + for (i = 0; i < ccount; i++) {
> + c = *pbuf;
> + pbuf++;
> + sp_write_reg(anx78xx, TX_P0, BUF_DATA_0 + i, c);
> +
> + if (i >= 15)
> + break;


So far as I can see after a brief look at it, ccount is always 1 so we
will never hit the i >= 15 condition. If we did though, then shouldn't
we handle it at the start of the function? I never know how to handle
these imaginary situations in the right way...

> + }
> + sp_write_reg_or(anx78xx, TX_P0, AUX_CTRL2, AUX_OP_EN);
> + sp_wait_aux_op_finish(anx78xx, &ret);
> + return ret;
> +}
> +
> +static u8 sp_tx_aux_dpcdwrite_byte(struct anx78xx *anx78xx, u8 addrh,
> + u8 addrm, u8 addrl, u8 data1)
> +{
> + u8 ret;
> +
> + sp_write_reg(anx78xx, TX_P0, AUX_CTRL, 0x08);
> + write_dpcd_addr(anx78xx, addrh, addrm, addrl);
> + sp_write_reg(anx78xx, TX_P0, BUF_DATA_0, data1);
> + sp_write_reg_or(anx78xx, TX_P0, AUX_CTRL2, AUX_OP_EN);
> + sp_wait_aux_op_finish(anx78xx, &ret);
> + return ret;
> +}
> +
> +static void sp_block_power_ctrl(struct anx78xx *anx78xx,
> + enum sp_tx_power_block sp_tx_pd_block, u8 power)
> +{
> + struct device *dev = &anx78xx->client->dev;
> +
> + if (power == SP_POWER_ON)
> + sp_write_reg_and(anx78xx, TX_P2, SP_POWERD_CTRL_REG,
> + ~sp_tx_pd_block);
> + else
> + sp_write_reg_or(anx78xx, TX_P2, SP_POWERD_CTRL_REG,
> + sp_tx_pd_block);
> +
> + dev_dbg(dev, "sp_tx_power_on: %.2x\n", (u16)sp_tx_pd_block);
> +}
> +
> +static void sp_vbus_power_off(struct anx78xx *anx78xx)
> +{
> + struct device *dev = &anx78xx->client->dev;
> + int i;
> +
> + for (i = 0; i < 5; i++) {
> + sp_write_reg_and(anx78xx, TX_P2, TX_PLL_FILTER5,
> + (~P5V_PROTECT_PD & ~SHORT_PROTECT_PD));
> + sp_write_reg_or(anx78xx, TX_P2, TX_PLL_FILTER, V33_SWITCH_ON);
> + if (!(sp_i2c_read_byte(anx78xx, TX_P2, TX_PLL_FILTER5)
> + & 0xc0)) {
> + dev_dbg(dev, "3.3V output enabled\n");
> + break;
> + }
> +
> + dev_dbg(dev, "VBUS power can not be supplied\n");

Why print this five times?

> + }
> +}
> +
> +void sp_tx_clean_state_machine(void)
> +{
> + sp.tx_system_state = STATE_WAITTING_CABLE_PLUG;
> + sp.tx_system_state_bak = STATE_WAITTING_CABLE_PLUG;
> + sp.tx_sc_state = SC_INIT;
> + sp.tx_lt_state = LT_INIT;
> + sp.hcdp_state = HDCP_CAPABLE_CHECK;
> + sp.tx_vo_state = VO_WAIT_VIDEO_STABLE;
> + sp.tx_ao_state = AO_INIT;
> +}
> +
> +u8 sp_tx_cur_states(void)
> +{
> + return sp.tx_system_state;
> +}
> +
> +void sp_tx_variable_init(void)
> +{
> + u16 i;
> +
> + sp.block_en = 1;
> +
> + sp.tx_system_state = STATE_WAITTING_CABLE_PLUG;
> + sp.tx_system_state_bak = STATE_WAITTING_CABLE_PLUG;
> +
> + sp.edid_break = 0;
> + sp.read_edid_flag = 0;
> + sp.edid_checksum = 0;
> + for (i = 0; i < 256; i++)
> + sp.edid_blocks[i] = 0;
> +
> + sp.tx_lt_state = LT_INIT;
> + sp.hcdp_state = HDCP_CAPABLE_CHECK;
> + sp.need_clean_status = 0;
> + sp.tx_sc_state = SC_INIT;
> + sp.tx_vo_state = VO_WAIT_VIDEO_STABLE;
> + sp.tx_ao_state = AO_INIT;
> + sp.changed_bandwidth = LINK_5P4G;
> + sp.hdmi_dvi_status = HDMI_MODE;
> +
> + sp.tx_test_lt = 0;
> + sp.tx_test_bw = 0;
> + sp.tx_test_edid = 0;
> +
> + sp.ds_vid_stb_cntr = 0;
> + sp.hdcp_fail_count = 0;
> +
> +}
> +
> +static void hdmi_rx_tmds_phy_initialization(struct anx78xx *anx78xx)
> +{
> + sp_write_reg(anx78xx, RX_P0, HDMI_RX_TMDS_CTRL_REG2, 0xa9);
> + sp_write_reg(anx78xx, RX_P0, HDMI_RX_TMDS_CTRL_REG7, 0x80);
> +
> + sp_write_reg(anx78xx, RX_P0, HDMI_RX_TMDS_CTRL_REG1, 0x90);
> + sp_write_reg(anx78xx, RX_P0, HDMI_RX_TMDS_CTRL_REG6, 0x92);
> + sp_write_reg(anx78xx, RX_P0, HDMI_RX_TMDS_CTRL_REG20, 0xf2);
> +}
> +
> +static void hdmi_rx_initialization(struct anx78xx *anx78xx)
> +{
> + struct device *dev = &anx78xx->client->dev;
> +
> + sp_write_reg(anx78xx, RX_P0, RX_MUTE_CTRL, AUD_MUTE | VID_MUTE);
> + sp_write_reg_or(anx78xx, RX_P0, RX_CHIP_CTRL,
> + MAN_HDMI5V_DET | PLLLOCK_CKDT_EN | DIGITAL_CKDT_EN);
> +
> + sp_write_reg_or(anx78xx, RX_P0, RX_SRST, HDCP_MAN_RST | SW_MAN_RST |
> + TMDS_RST | VIDEO_RST);
> + sp_write_reg_and(anx78xx, RX_P0, RX_SRST, ~HDCP_MAN_RST &
> + ~SW_MAN_RST & ~TMDS_RST & ~VIDEO_RST);
> +
> + sp_write_reg_or(anx78xx, RX_P0, RX_AEC_EN0, AEC_EN06 | AEC_EN05);
> + sp_write_reg_or(anx78xx, RX_P0, RX_AEC_EN2, AEC_EN21);
> + sp_write_reg_or(anx78xx, RX_P0, RX_AEC_CTRL, AVC_EN | AAC_OE | AAC_EN);
> +
> + sp_write_reg_and(anx78xx, RX_P0, RX_SYS_PWDN1, ~PWDN_CTRL);
> +
> + sp_write_reg_or(anx78xx, RX_P0, RX_VID_DATA_RNG, R2Y_INPUT_LIMIT);
> + sp_write_reg(anx78xx, RX_P0, 0x65, 0xc4);
> + sp_write_reg(anx78xx, RX_P0, 0x66, 0x18);
> +
> + /* enable DDC stretch */
> + sp_write_reg(anx78xx, TX_P0, TX_EXTRA_ADDR, 0x50);
> +
> + hdmi_rx_tmds_phy_initialization(anx78xx);
> + hdmi_rx_set_hpd(anx78xx, 0);
> + hdmi_rx_set_termination(anx78xx, 0);
> + dev_dbg(dev, "HDMI Rx is initialized...\n");

Delete. Use ftrace.

> +}
> +
> +struct anx78xx_clock_data const pxtal_data[XTAL_CLK_NUM] = {
> + {19, 192},
> + {24, 240},
> + {25, 250},
> + {26, 260},
> + {27, 270},
> + {38, 384},
> + {52, 520},
> + {27, 270},
> +};
> +
> +static void xtal_clk_sel(struct anx78xx *anx78xx)
> +{
> + struct device *dev = &anx78xx->client->dev;
> +
> + dev_dbg(dev, "define XTAL_CLK: %x\n ", (u16)XTAL_27M);
> + sp_write_reg_and_or(anx78xx, TX_P2,
> + TX_ANALOG_DEBUG2, (~0x3c), 0x3c & (XTAL_27M << 2));
> + sp_write_reg(anx78xx, TX_P0, 0xEC, (u8)(((u16)XTAL_CLK_M10)));

Remove the superflous casts to u16.

> + sp_write_reg(anx78xx, TX_P0, 0xED,
> + (u8)(((u16)XTAL_CLK_M10 & 0xFF00) >> 2) | XTAL_CLK);
> +
> + sp_write_reg(anx78xx, TX_P0,
> + I2C_GEN_10US_TIMER0, (u8)(((u16)XTAL_CLK_M10)));
> + sp_write_reg(anx78xx, TX_P0, I2C_GEN_10US_TIMER1,
> + (u8)(((u16)XTAL_CLK_M10 & 0xFF00) >> 8));
> + sp_write_reg(anx78xx, TX_P0, 0xBF, (u8)(((u16)XTAL_CLK - 1)));
> +
> + sp_write_reg_and_or(anx78xx, RX_P0, 0x49, 0x07,
> + (u8)(((((u16)XTAL_CLK) >> 1) - 2) << 3));
> +
> +}
> +
> +void sp_tx_initialization(struct anx78xx *anx78xx)
> +{
> + sp_write_reg(anx78xx, TX_P0, AUX_CTRL2, 0x30);
> + sp_write_reg_or(anx78xx, TX_P0, AUX_CTRL2, 0x08);
> +
> + sp_write_reg_and(anx78xx, TX_P0, TX_HDCP_CTRL,
> + (u8)(~AUTO_EN) & (~AUTO_START));
> + sp_write_reg(anx78xx, TX_P0, OTP_KEY_PROTECT1, OTP_PSW1);
> + sp_write_reg(anx78xx, TX_P0, OTP_KEY_PROTECT2, OTP_PSW2);
> + sp_write_reg(anx78xx, TX_P0, OTP_KEY_PROTECT3, OTP_PSW3);
> + sp_write_reg_or(anx78xx, TX_P0, HDCP_KEY_CMD, DISABLE_SYNC_HDCP);
> + sp_write_reg(anx78xx, TX_P2, SP_TX_VID_CTRL8_REG, VID_VRES_TH);
> +
> + sp_write_reg(anx78xx, TX_P0, HDCP_AUTO_TIMER, HDCP_AUTO_TIMER_VAL);
> + sp_write_reg_or(anx78xx, TX_P0, TX_HDCP_CTRL, LINK_POLLING);
> +
> + sp_write_reg_or(anx78xx, TX_P0, TX_LINK_DEBUG, M_VID_DEBUG);
> + sp_write_reg_or(anx78xx, TX_P2, TX_ANALOG_DEBUG2, POWERON_TIME_1P5MS);
> +
> + xtal_clk_sel(anx78xx);
> + sp_write_reg(anx78xx, TX_P0, AUX_DEFER_CTRL, 0x8C);
> +
> + sp_write_reg_or(anx78xx, TX_P0, TX_DP_POLLING, AUTO_POLLING_DISABLE);
> + /*
> + * Short the link intergrity check timer to speed up bstatus
> + * polling for HDCP CTS item 1A-07
> + */
> + sp_write_reg(anx78xx, TX_P0, SP_TX_LINK_CHK_TIMER, 0x1d);
> + sp_write_reg_or(anx78xx, TX_P0, TX_MISC, EQ_TRAINING_LOOP);
> +
> + sp_write_reg_or(anx78xx, TX_P0, SP_TX_ANALOG_PD_REG, CH0_PD);
> +
> + sp_write_reg(anx78xx, TX_P2, SP_TX_INT_CTRL_REG, 0X01);
> + /* disable HDCP mismatch function for VGA dongle */
> + sp_tx_link_phy_initialization(anx78xx);
> + gen_m_clk_with_downspeading(anx78xx);
> +
> + sp.down_sample_en = 0;
> +}
> +
> +bool sp_chip_detect(struct anx78xx *anx78xx)
> +{
> + struct device *dev = &anx78xx->client->dev;
> + u16 id;
> + u8 idh = 0, idl = 0;
> + int i;
> +
> + anx78xx_poweron(anx78xx);
> +
> + /* check chip id */
> + sp_read_reg(anx78xx, TX_P2, SP_TX_DEV_IDL_REG, &idl);
> + sp_read_reg(anx78xx, TX_P2, SP_TX_DEV_IDH_REG, &idh);
> + id = idl | (idh << 8);
> +
> + dev_dbg(dev, "CHIPID: ANX%x\n", id & 0xffff);
> +
> + for (i = 0; i < sizeof(chipid_list) / sizeof(u16); i++) {

for (i = 0; i < ARRAY_SIZE(chipid_list); i++) {

> + if (id == chipid_list[i])
> + return true;
> + }
> +
> + return false;
> +}
> +
> +static void sp_waiting_cable_plug_process(struct anx78xx *anx78xx)
> +{
> + sp_tx_variable_init();
> + anx78xx_poweron(anx78xx);
> + goto_next_system_state(anx78xx);
> +}
> +
> +/*
> + * Check if it is ANALOGIX dongle.
> + */
> +static const u8 ANX_OUI[3] = {0x00, 0x22, 0xB9};
> +
> +static u8 is_anx_dongle(struct anx78xx *anx78xx)
> +{
> + u8 buf[3];
> +
> + /* 0x0500~0x0502: BRANCH_IEEE_OUI */
> + sp_tx_aux_dpcdread_bytes(anx78xx, 0x00, 0x05, 0x00, 3, buf);
> +
> + if (!memcmp(buf, ANX_OUI, 3))
> + return 1;
> +
> + return 0;
> +}
> +
> +static void sp_tx_get_rx_bw(struct anx78xx *anx78xx, u8 *bw)
> +{
> + if (is_anx_dongle(anx78xx))
> + *bw = LINK_6P75G; /* just for debug */
> + else
> + sp_tx_aux_dpcdread_bytes(anx78xx, 0x00, 0x00,
> + DPCD_MAX_LINK_RATE, 1, bw);
> +}
> +
> +static u8 sp_tx_get_cable_type(struct anx78xx *anx78xx,
> + enum cable_type_status det_cable_type_state)
> +{
> + struct device *dev = &anx78xx->client->dev;
> +
> + u8 ds_port_preset;
> + u8 aux_status;
> + u8 data_buf[16];
> + u8 cur_cable_type;
> +
> + ds_port_preset = 0;
> + cur_cable_type = DWN_STRM_IS_NULL;
> +
> + aux_status = sp_tx_aux_dpcdread_bytes(anx78xx, 0x00, 0x00, 0x05, 1,
> + &ds_port_preset);
> +
> + dev_dbg(dev, "DPCD 0x005: %x\n", (int)ds_port_preset);
> +
> + switch (det_cable_type_state) {
> + case CHECK_AUXCH:
> + if (AUX_OK == aux_status) {
> + sp_tx_aux_dpcdread_bytes(anx78xx, 0x00, 0x00, 0, 0x0c,
> + data_buf);
> + det_cable_type_state = GETTED_CABLE_TYPE;
> + } else {
> + dev_err(dev, " AUX access error\n");
> + break;
> + }
> + case GETTED_CABLE_TYPE:
> + switch ((ds_port_preset & (BIT(1) | BIT(2))) >> 1) {

Extra space char.

switch ((ds_port_preset & (BIT(1) | BIT(2))) >> 1) {

> + case 0x00:
> + cur_cable_type = DWN_STRM_IS_DIGITAL;
> + dev_dbg(dev, "Downstream is DP dongle.\n");
> + break;
> + case 0x01:
> + case 0x03:
> + cur_cable_type = DWN_STRM_IS_ANALOG;
> + dev_dbg(dev, "Downstream is VGA dongle.\n");
> + break;
> + case 0x02:
> + cur_cable_type = DWN_STRM_IS_HDMI;
> + dev_dbg(dev, "Downstream is HDMI dongle.\n");
> + break;
> + default:
> + cur_cable_type = DWN_STRM_IS_NULL;
> + dev_err(dev, "Downstream can not recognized.\n");
> + break;
> + }
> + default:
> + break;
> + }
> + return cur_cable_type;
> +}
> +
> +static u8 sp_tx_get_dp_connection(struct anx78xx *anx78xx)
> +{
> + u8 c;
> +
> + if (AUX_OK != sp_tx_aux_dpcdread_bytes(anx78xx, 0x00, 0x02,
> + DPCD_SINK_COUNT, 1, &c))
> + return 0;
> +
> + if (c & 0x1f) {
> + sp_tx_aux_dpcdread_bytes(anx78xx, 0x00, 0x00, 0x04, 1, &c);
> + if (c & 0x20) {
> + sp_tx_aux_dpcdread_bytes(anx78xx, 0x00, 0x06, 0x00, 1,
> + &c);
> + /*
> + * Bit 5 = SET_DN_DEVICE_DP_PWR_5V
> + * Bit 6 = SET_DN_DEVICE_DP_PWR_12V
> + * Bit 7 = SET_DN_DEVICE_DP_PWR_18V
> + */
> + c = c & 0x1F;
> + sp_tx_aux_dpcdwrite_byte(anx78xx, 0x00, 0x06, 0x00,
> + c | 0x20);
> + }
> + return 1;
> + } else
> + return 0;
> +}
> +
> +static u8 sp_tx_get_downstream_connection(struct anx78xx *anx78xx)
> +{
> + return sp_tx_get_dp_connection(anx78xx);
> +}
> +
> +static void sp_sink_connection(struct anx78xx *anx78xx)
> +{
> + struct device *dev = &anx78xx->client->dev;
> +
> + switch (sp.tx_sc_state) {
> + case SC_INIT:
> + sp.tx_sc_state++;
> + case SC_CHECK_CABLE_TYPE:
> + case SC_WAITTING_CABLE_TYPE:
> + default:
> + if (sp_tx_get_cable_type(anx78xx, CHECK_AUXCH) ==
> + DWN_STRM_IS_NULL) {
> + sp.tx_sc_state++;
> + if (sp.tx_sc_state >= SC_WAITTING_CABLE_TYPE) {
> + sp.tx_sc_state = SC_NOT_CABLE;
> + dev_dbg(dev, "Can not get cable type!\n");
> + }
> + break;
> + }
> +
> + sp.tx_sc_state = SC_SINK_CONNECTED;
> + case SC_SINK_CONNECTED:
> + if (sp_tx_get_downstream_connection(anx78xx))
> + goto_next_system_state(anx78xx);
> + break;
> + case SC_NOT_CABLE:
> + sp_vbus_power_off(anx78xx);
> + reg_hardware_reset(anx78xx);
> + break;
> + }
> +}
> +
> +/******************start EDID process********************/
> +static void sp_tx_enable_video_input(struct anx78xx *anx78xx, u8 enable)
> +{
> + struct device *dev = &anx78xx->client->dev;
> + u8 c;
> +
> + sp_read_reg(anx78xx, TX_P2, VID_CTRL1, &c);
> + if (enable) {
> + c = (c & 0xf7) | VIDEO_EN;
> + sp_write_reg(anx78xx, TX_P2, VID_CTRL1, c);
> + dev_dbg(dev, "Slimport Video is enabled!\n");
> +
> + } else {
> + c &= ~VIDEO_EN;
> + sp_write_reg(anx78xx, TX_P2, VID_CTRL1, c);
> + dev_dbg(dev, "Slimport Video is disabled!\n");
> + }
> +}
> +
> +static u8 sp_get_edid_detail(u8 *data_buf)
> +{
> + u16 pixclock_edid;
> +
> + pixclock_edid = ((((u16)data_buf[1] << 8))
> + | ((u16)data_buf[0] & 0xFF));
> + if (pixclock_edid <= 5300)
> + return LINK_1P62G;
> + else if ((pixclock_edid > 5300) && (pixclock_edid <= 8900))
> + return LINK_2P7G;
> + else if ((pixclock_edid > 8900) && (pixclock_edid <= 18000))
> + return LINK_5P4G;
> + else
> + return LINK_6P75G;
> +}
> +
> +static u8 sp_parse_edid_to_get_bandwidth(struct anx78xx *anx78xx)
> +{
> + struct device *dev = &anx78xx->client->dev;
> + u8 desc_offset = 0;
> + u8 i, bandwidth, temp;
> +
> + bandwidth = LINK_1P62G;
> + temp = LINK_1P62G;
> + i = 0;
> + while (i < 4 && sp.edid_blocks[0x36 + desc_offset] != 0) {
> + temp = sp_get_edid_detail(sp.edid_blocks + 0x36 + desc_offset);
> + dev_dbg(dev, "bandwidth via EDID : %x\n", (u16)temp);
> + if (bandwidth < temp)
> + bandwidth = temp;
> + if (bandwidth > LINK_5P4G)
> + break;
> + desc_offset += 0x12;
> + ++i;
> + }
> + return bandwidth;
> +}
> +
> +static void sp_tx_aux_wr(struct anx78xx *anx78xx, u8 offset)
> +{
> + sp_write_reg(anx78xx, TX_P0, BUF_DATA_0, offset);
> + sp_write_reg(anx78xx, TX_P0, AUX_CTRL, 0x04);
> + sp_write_reg_or(anx78xx, TX_P0, AUX_CTRL2, AUX_OP_EN);
> + sp_wait_aux_op_finish(anx78xx, &sp.edid_break);
> +}
> +
> +static void sp_tx_aux_rd(struct anx78xx *anx78xx, u8 len_cmd)
> +{
> + sp_write_reg(anx78xx, TX_P0, AUX_CTRL, len_cmd);
> + sp_write_reg_or(anx78xx, TX_P0, AUX_CTRL2, AUX_OP_EN);
> + sp_wait_aux_op_finish(anx78xx, &sp.edid_break);
> +}
> +
> +static u8 sp_tx_get_edid_block(struct anx78xx *anx78xx)
> +{
> + struct device *dev = &anx78xx->client->dev;
> + u8 c;
> +
> + sp_tx_aux_wr(anx78xx, 0x7e);
> + sp_tx_aux_rd(anx78xx, 0x01);
> + sp_read_reg(anx78xx, TX_P0, BUF_DATA_0, &c);
> + dev_dbg(dev, "EDID Block = %d\n", (int)(c + 1));
> +
> + if (c > 3)
> + c = 1;
> + return c;
> +}
> +
> +static void sp_edid_read(struct anx78xx *anx78xx, u8 offset,
> + u8 *pblock_buf)
> +{
> + u8 data_cnt, cnt;
> + u8 c;
> +
> + sp_tx_aux_wr(anx78xx, offset);
> + sp_tx_aux_rd(anx78xx, 0xf5);
> + data_cnt = 0;
> + cnt = 0;
> +
> + while ((data_cnt) < 16) {
> + sp_read_reg(anx78xx, TX_P0, BUF_DATA_COUNT, &c);
> +
> + if ((c & 0x1f) != 0) {

Double negative. The right times to compare == 0 and != 0 are with
*cmp() functions and when talking about zero as a number. Here zero is
a boolean so it should just be "if (c & 0x1f) {"

> + data_cnt = data_cnt + c;
> + do {
> + sp_read_reg(anx78xx, TX_P0, BUF_DATA_0 + c - 1,
> + &(pblock_buf[c - 1]));
> + if (c == 1)
> + break;
> + } while (c--);

The "if (c == 1)" condition means that c is a number 2-255 so this
"while (c--)" condition is always true. Remove the earlier condition
and do this instead:

} while (--c)

Anyway, gotta run.

regards,
dan carpenter

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/