Messages in this thread | | | Date | Fri, 28 Jul 2023 00:30:53 +0200 | Subject | Re: [PATCH] hid-mcp2200: added driver for GPIOs of MCP2200 | From | Christophe JAILLET <> |
| |
Le 23/06/2023 à 13:01, Johannes Roith a écrit : > Added a gpiochip compatible driver to control the 8 GPIOs of the MCP2200 > by using the HID interface. > > Using GPIOs with alternative functions (GP0<->SSPND, GP1<->USBCFG, > GP6<->RXLED, GP7<->TXLED) will reset the functions, if set (unset by > default). > > The driver was tested while also using the UART of the chip. Setting > and reading the GPIOs has no effect on the UART communication. However, > a reset is triggered after the CONFIGURE command. If the GPIO Direction > is constantly changed, this will affect the communication at low baud > rates. This is a hardware problem of the MCP2200 and is not caused by > the driver. > > Feedback from reviewers Christophe JAILLET <christophe.jaillet@wanadoo.fr> > and Andi Shyti <andi.shyti@kernel.org> was added. > > Signed-off-by: Johannes Roith <johannes@gnu-linux.rocks> > ---
[...]
> +static int mcp2200_probe(struct hid_device *hdev, const struct hid_device_id *id) > +{ > + int ret; > + struct mcp2200 *mcp; > + > + mcp = devm_kzalloc(&hdev->dev, sizeof(*mcp), GFP_KERNEL); > + if (!mcp) > + return -ENOMEM; > + > + ret = hid_parse(hdev); > + if (ret) { > + hid_err(hdev, "can't parse reports\n"); > + return ret; > + } > + > + /* > + * This driver uses the .raw_event callback and therefore does not need any > + * HID_CONNECT_xxx flags. > + */ > + ret = hid_hw_start(hdev, 0); > + if (ret) { > + hid_err(hdev, "can't start hardware\n"); > + return ret; > + } > + > + hid_info(hdev, "USB HID v%x.%02x Device [%s] on %s\n", hdev->version >> 8, > + hdev->version & 0xff, hdev->name, hdev->phys); > + > + ret = hid_hw_open(hdev); > + if (ret) { > + hid_err(hdev, "can't open device\n"); > + hid_hw_stop(hdev); > + return ret; > + } > + > + mutex_init(&mcp->lock); > + init_completion(&mcp->wait_in_report); > + hid_set_drvdata(hdev, mcp); > + mcp->hdev = hdev; > + > + ret = devm_add_action_or_reset(&hdev->dev, mcp2200_hid_unregister, hdev); > + if (ret) > + return ret; > + > + mcp->gc = template_chip; > + mcp->gc.parent = &hdev->dev; > + > + ret = gpiochip_add_data(&mcp->gc, mcp);
devm_gpiochip_add_data()?
> + if (ret < 0) { > + dev_err(&hdev->dev, "Unable to register gpiochip\n");
hid_err() to be consistent?
> + hid_hw_stop(hdev);
Isn't it already run by mcp2200_hid_unregister() registered a few lines above?
CJ
> + return ret; > + } > + > + return 0; > +} > + > +static void mcp2200_remove(struct hid_device *hdev) > +{ > + struct mcp2200 *mcp; > + > + mcp = hid_get_drvdata(hdev); > + gpiochip_remove(&mcp->gc); > +} > +
[...]
| |