On 28/07/2022 14:52, Julien Panis wrote:
> ECAP hardware on AM62x SoC supports capture feature. It can be used
> to timestamp events (falling/rising edges) detected on signal input pin.
>
> This commit adds capture driver support for ECAP hardware on AM62x SoC.
>
> In the ECAP hardware, capture pin can also be configured to be in
Thank you for your patch. There is something to discuss/improve.
(...)
> +static int ecap_iio_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct ecap_iio_dev *ecap_dev;
> + struct iio_dev *indio_dev;
> + void __iomem *mmio_base;
> + int ret;
> +
> + indio_dev = devm_iio_device_alloc(dev, sizeof(*ecap_dev));
> + if (IS_ERR(indio_dev)) {
> + dev_err(dev, "failed to allocate memory for iio device\n");
Do not print messages, which core takes care of.
> + return PTR_ERR(indio_dev);
> + }
> +
> + ecap_dev = iio_priv(indio_dev);
> +
> + ecap_dev->clk = devm_clk_get(dev, "fck");
> + if (IS_ERR(ecap_dev->clk)) {
> + dev_err(dev, "failed to get clock\n");
> + return PTR_ERR(ecap_dev->clk);
> + }
> +
> + ret = clk_prepare_enable(ecap_dev->clk);
> + if (ret) {
> + dev_err(dev, "failed to enable clock\n");
> + return ret;
> + }
> +
> + ret = devm_add_action_or_reset(dev, ecap_iio_clk_disable, ecap_dev->clk);
> + if (ret) {
> + dev_err(dev, "failed to add clock disable action\n");
> + return ret;
> + }
> +
> + ecap_dev->clk_rate = clk_get_rate(ecap_dev->clk);
> + if (!ecap_dev->clk_rate) {
> + dev_err(dev, "failed to get clock rate\n");
> + return -EINVAL;
> + }
> +
> + if (prescaler > ECAP_PS_MAX_VAL) {
> + prescaler = ECAP_PS_MAX_VAL;
> + dev_warn(dev, "prescaler out of range, forced to %d\n", prescaler);
> + }
> +
> + mmio_base = devm_platform_ioremap_resource(pdev, 0);
> + if (IS_ERR(mmio_base)) {
> + dev_err(dev, "failed to remap io\n");
No need for msg.
> + return PTR_ERR(mmio_base);
> + }
> +
> + ecap_dev->regmap = regmap_init_mmio(dev, mmio_base, &ecap_iio_regmap_config);
> + if (IS_ERR(ecap_dev->regmap)) {
> + dev_err(dev, "failed to init regmap\n");
> + return PTR_ERR(ecap_dev->regmap);
> + }
> +
> + indio_dev->name = devm_kasprintf(dev, GFP_KERNEL,
> + "ecap-iio-%p", mmio_base);
> + indio_dev->info = &ecap_iio_info;
> + indio_dev->channels = ecap_iio_channels;
> + indio_dev->num_channels = ARRAY_SIZE(ecap_iio_channels);
> + indio_dev->modes = INDIO_BUFFER_SOFTWARE;
> +
> + ret = devm_iio_kfifo_buffer_setup_ext(dev, indio_dev, NULL, NULL);
> + if (ret) {
> + dev_err(dev, "failed to setup iio buffer\n");
> + return ret;
> + }
> +
> + ret = platform_get_irq(pdev, 0);
> + if (ret < 0) {
> + dev_err(dev, "failed to get irq\n");
> + return ret;
> + }
> +
> + ret = devm_request_irq(dev, ret, ecap_iio_isr, 0, pdev->name, indio_dev);
> + if (ret) {
> + dev_err(dev, "failed to request irq\n");
> + return ret;
> + }
Best regards,
Krzysztof
