On Tue, Jan 22, 2019 at 02:27:51PM +0900, Chanwoo Choi wrote:
> 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
Thanks, I will rebase and share an updated patchset.
>
> >
> > 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,
> >> ®_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,
> >> ®_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,
> >> ®_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, ®_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
