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 devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html