Re: [PATCH] drivers: extcon: Add support for ptn5150

From: Chanwoo Choi
Date: Tue Jan 22 2019 - 00:28:06 EST

Hi Vijai,

On 19. 1. 22. ìí 1:42, Vijai Kumar K wrote:
> Hi Chanwoo Choi,
>
> This is the first time I am sending a driver to LKML. I have a few doubts. Can
> you please clarify them when you are free?
>
> 1. I have developed and tested this patch on 4.14.89 kernel. When trying to
> mainline the driver should I rebase and send the patch on top of current tree(v5.0-rc2)?

You better to rebase your patch on extcon-next branch (v5.0-rc1).
because the extcon.git[1] keep the v5.0-rc1 version. But, if you use over v5.0-rc1 version,
it doesn't matter. Maybe, this patch doesn't get the any impact from the modification
of v5.0-rcX.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/extcon.git

>
> 2. Is there any specific git on which I should test this driver on? Like below?
> https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/extcon.git

Yes.

>
> Thanks,
> Vijai Kumar K
>
> On Tue, Jan 22, 2019 at 08:05:33AM +0800, kbuild test robot wrote:
>> Hi Vijai,
>>
>> Thank you for the patch! Yet something to improve:
>>
>> [auto build test ERROR on chanwoo-extcon/extcon-next]
>> [also build test ERROR on v5.0-rc2]
>> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
>>
>> url: https://github.com/0day-ci/linux/commits/Vijai-Kumar-K/drivers-extcon-Add-support-for-ptn5150/20190122-041153
>> base: https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/extcon.git extcon-next
>> config: sh-allyesconfig (attached as .config)
>> compiler: sh4-linux-gnu-gcc (Debian 8.2.0-11) 8.2.0
>> reproduce:
>> wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>> chmod +x ~/bin/make.cross
>> # save the attached .config to linux build tree
>> GCC_VERSION=8.2.0 make.cross ARCH=sh
>>
>> All error/warnings (new ones prefixed by >>):
>>
>> drivers//extcon/extcon-ptn5150.c: In function 'ptn5150_irq_work':
>>>> drivers//extcon/extcon-ptn5150.c:93:5: error: implicit declaration of function 'extcon_set_state_sync'; did you mean 'extcon_get_state'? [-Werror=implicit-function-declaration]
>> extcon_set_state_sync(info->edev,
>> ^~~~~~~~~~~~~~~~~~~~~
>> extcon_get_state
>> drivers//extcon/extcon-ptn5150.c: In function 'ptn5150_i2c_probe':
>>>> drivers//extcon/extcon-ptn5150.c:255:15: error: implicit declaration of function 'devm_extcon_dev_allocate'; did you mean 'extcon_get_state'? [-Werror=implicit-function-declaration]
>> info->edev = devm_extcon_dev_allocate(info->dev, ptn5150_extcon_cable);
>> ^~~~~~~~~~~~~~~~~~~~~~~~
>> extcon_get_state
>>>> drivers//extcon/extcon-ptn5150.c:255:13: warning: assignment to 'struct extcon_dev *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
>> info->edev = devm_extcon_dev_allocate(info->dev, ptn5150_extcon_cable);
>> ^
>>>> drivers//extcon/extcon-ptn5150.c:262:8: error: implicit declaration of function 'devm_extcon_dev_register'; did you mean 'devm_pinctrl_register'? [-Werror=implicit-function-declaration]
>> ret = devm_extcon_dev_register(info->dev, info->edev);
>> ^~~~~~~~~~~~~~~~~~~~~~~~
>> devm_pinctrl_register
>> cc1: some warnings being treated as errors
>>
>> vim +93 drivers//extcon/extcon-ptn5150.c
>>
>> 51
>> 52 static void ptn5150_irq_work(struct work_struct *work)
>> 53 {
>> 54 struct ptn5150_info *info = container_of(work,
>> 55 struct ptn5150_info, irq_work);
>> 56 int ret = 0;
>> 57 unsigned int reg_data;
>> 58 unsigned int port_status;
>> 59 unsigned int vbus;
>> 60 unsigned int cable_attach;
>> 61 unsigned int int_status;
>> 62
>> 63 if (!info->edev)
>> 64 return;
>> 65
>> 66 mutex_lock(&info->mutex);
>> 67
>> 68 ret = regmap_read(info->regmap, PTN5150_REG_CC_STATUS, &reg_data);
>> 69 if (ret) {
>> 70 dev_err(info->dev,
>> 71 "failed to read CC STATUS %d\n", ret);
>> 72 mutex_unlock(&info->mutex);
>> 73 return;
>> 74 }
>> 75 /* Clear interrupt. Read would clear the register */
>> 76 ret = regmap_read(info->regmap, PTN5150_REG_INT_STATUS, &int_status);
>> 77 if (ret) {
>> 78 dev_err(info->dev,
>> 79 "failed to read INT STATUS %d\n", ret);
>> 80 mutex_unlock(&info->mutex);
>> 81 return;
>> 82 }
>> 83
>> 84 if (int_status) {
>> 85 cable_attach = int_status & PTN5150_REG_INT_CABLE_ATTACH_MASK;
>> 86 if (cable_attach) {
>> 87 port_status = ((reg_data &
>> 88 PTN5150_REG_CC_PORT_ATTACHMENT_MASK) >>
>> 89 PTN5150_REG_CC_PORT_ATTACHMENT_SHIFT);
>> 90
>> 91 switch (port_status) {
>> 92 case PTN5150_DFP_ATTACHED:
>> > 93 extcon_set_state_sync(info->edev,
>> 94 EXTCON_USB_HOST, false);
>> 95 gpiod_set_value(info->vbus_gpiod, 0);
>> 96 extcon_set_state_sync(info->edev, EXTCON_USB,
>> 97 true);
>> 98 break;
>> 99 case PTN5150_UFP_ATTACHED:
>> 100 extcon_set_state_sync(info->edev, EXTCON_USB,
>> 101 false);
>> 102 vbus = ((reg_data &
>> 103 PTN5150_REG_CC_VBUS_DETECTION_MASK) >>
>> 104 PTN5150_REG_CC_VBUS_DETECTION_SHIFT);
>> 105 if (vbus)
>> 106 gpiod_set_value(info->vbus_gpiod, 0);
>> 107 else
>> 108 gpiod_set_value(info->vbus_gpiod, 1);
>> 109
>> 110 extcon_set_state_sync(info->edev,
>> 111 EXTCON_USB_HOST, true);
>> 112 break;
>> 113 default:
>> 114 dev_err(info->dev,
>> 115 "Unknown Port status : %x\n",
>> 116 port_status);
>> 117 break;
>> 118 }
>> 119 } else {
>> 120 extcon_set_state_sync(info->edev,
>> 121 EXTCON_USB_HOST, false);
>> 122 extcon_set_state_sync(info->edev,
>> 123 EXTCON_USB, false);
>> 124 gpiod_set_value(info->vbus_gpiod, 0);
>> 125 }
>> 126 }
>> 127
>> 128 /* Clear interrupt. Read would clear the register */
>> 129 ret = regmap_read(info->regmap, PTN5150_REG_INT_REG_STATUS,
>> 130 &int_status);
>> 131 if (ret) {
>> 132 dev_err(info->dev,
>> 133 "failed to read INT REG STATUS %d\n", ret);
>> 134 mutex_unlock(&info->mutex);
>> 135 return;
>> 136 }
>> 137
>> 138
>> 139 mutex_unlock(&info->mutex);
>> 140 }
>> 141
>> 142
>> 143 static irqreturn_t ptn5150_irq_handler(int irq, void *data)
>> 144 {
>> 145 struct ptn5150_info *info = data;
>> 146
>> 147 schedule_work(&info->irq_work);
>> 148
>> 149 return IRQ_HANDLED;
>> 150 }
>> 151
>> 152 static int ptn5150_init_dev_type(struct ptn5150_info *info)
>> 153 {
>> 154 unsigned int reg_data, vendor_id, version_id;
>> 155 int ret;
>> 156
>> 157 ret = regmap_read(info->regmap, PTN5150_REG_DEVICE_ID, &reg_data);
>> 158 if (ret) {
>> 159 dev_err(info->dev, "failed to read DEVICE_ID %d\n", ret);
>> 160 return -EINVAL;
>> 161 }
>> 162
>> 163 vendor_id = ((reg_data & PTN5150_REG_DEVICE_ID_VENDOR_MASK) >>
>> 164 PTN5150_REG_DEVICE_ID_VENDOR_SHIFT);
>> 165 version_id = ((reg_data & PTN5150_REG_DEVICE_ID_VERSION_MASK) >>
>> 166 PTN5150_REG_DEVICE_ID_VERSION_SHIFT);
>> 167
>> 168 dev_info(info->dev, "Device type: version: 0x%x, vendor: 0x%x\n",
>> 169 version_id, vendor_id);
>> 170
>> 171 /* Clear any existing interrupts */
>> 172 ret = regmap_read(info->regmap, PTN5150_REG_INT_STATUS, &reg_data);
>> 173 if (ret) {
>> 174 dev_err(info->dev,
>> 175 "failed to read PTN5150_REG_INT_STATUS %d\n",
>> 176 ret);
>> 177 return -EINVAL;
>> 178 }
>> 179
>> 180 ret = regmap_read(info->regmap, PTN5150_REG_INT_REG_STATUS, &reg_data);
>> 181 if (ret) {
>> 182 dev_err(info->dev,
>> 183 "failed to read PTN5150_REG_INT_REG_STATUS %d\n", ret);
>> 184 return -EINVAL;
>> 185 }
>> 186
>> 187 return 0;
>> 188 }
>> 189
>> 190 static int ptn5150_i2c_probe(struct i2c_client *i2c,
>> 191 const struct i2c_device_id *id)
>> 192 {
>> 193 struct device *dev = &i2c->dev;
>> 194 struct device_node *np = i2c->dev.of_node;
>> 195 struct ptn5150_info *info;
>> 196 int ret;
>> 197
>> 198 if (!np)
>> 199 return -EINVAL;
>> 200
>> 201 info = devm_kzalloc(&i2c->dev, sizeof(*info), GFP_KERNEL);
>> 202 if (!info)
>> 203 return -ENOMEM;
>> 204 i2c_set_clientdata(i2c, info);
>> 205
>> 206 info->dev = &i2c->dev;
>> 207 info->i2c = i2c;
>> 208 info->int_gpiod = devm_gpiod_get(&i2c->dev, "int", GPIOD_IN);
>> 209 if (!info->int_gpiod) {
>> 210 dev_err(dev, "failed to get INT GPIO\n");
>> 211 return -EINVAL;
>> 212 }
>> 213 info->vbus_gpiod = devm_gpiod_get(&i2c->dev, "vbus", GPIOD_IN);
>> 214 if (!info->vbus_gpiod) {
>> 215 dev_err(dev, "failed to get VBUS GPIO\n");
>> 216 return -EINVAL;
>> 217 }
>> 218 ret = gpiod_direction_output(info->vbus_gpiod, 0);
>> 219 if (ret) {
>> 220 dev_err(dev, "failed to set VBUS GPIO direction\n");
>> 221 return -EINVAL;
>> 222 }
>> 223
>> 224 mutex_init(&info->mutex);
>> 225
>> 226 INIT_WORK(&info->irq_work, ptn5150_irq_work);
>> 227
>> 228 info->regmap = devm_regmap_init_i2c(i2c, &ptn5150_regmap_config);
>> 229 if (IS_ERR(info->regmap)) {
>> 230 ret = PTR_ERR(info->regmap);
>> 231 dev_err(info->dev, "failed to allocate register map: %d\n",
>> 232 ret);
>> 233 return ret;
>> 234 }
>> 235
>> 236 if (info->int_gpiod) {
>> 237 info->irq = gpiod_to_irq(info->int_gpiod);
>> 238 if (info->irq < 0) {
>> 239 dev_err(dev, "failed to get INTB IRQ\n");
>> 240 return info->irq;
>> 241 }
>> 242
>> 243 ret = devm_request_threaded_irq(dev, info->irq, NULL,
>> 244 ptn5150_irq_handler,
>> 245 IRQF_TRIGGER_FALLING |
>> 246 IRQF_ONESHOT,
>> 247 i2c->name, info);
>> 248 if (ret < 0) {
>> 249 dev_err(dev, "failed to request handler for INTB IRQ\n");
>> 250 return ret;
>> 251 }
>> 252 }
>> 253
>> 254 /* Allocate extcon device */
>> > 255 info->edev = devm_extcon_dev_allocate(info->dev, ptn5150_extcon_cable);
>> 256 if (IS_ERR(info->edev)) {
>> 257 dev_err(info->dev, "failed to allocate memory for extcon\n");
>> 258 return -ENOMEM;
>> 259 }
>> 260
>> 261 /* Register extcon device */
>> > 262 ret = devm_extcon_dev_register(info->dev, info->edev);
>> 263 if (ret) {
>> 264 dev_err(info->dev, "failed to register extcon device\n");
>> 265 return ret;
>> 266 }
>> 267
>> 268 /* Initialize PTN5150 device and print vendor id and version id */
>> 269 ret = ptn5150_init_dev_type(info);
>> 270 if (ret)
>> 271 return -EINVAL;
>> 272
>> 273 return 0;
>> 274 }
>> 275
>>
>> ---
>> 0-DAY kernel test infrastructure Open Source Technology Center
>> https://lists.01.org/pipermail/kbuild-all Intel Corporation
>
>
>
>


--
Best Regards,
Chanwoo Choi
Samsung Electronics