Hi Biju,
On Fri, Jul 29, 2022 at 2:25 PM Biju Das <biju.das.jz@xxxxxxxxxxxxxx> wrote:
> The output pins of the RZ/G2L general PWM timer (GPT) can be disabled
> by using the port output enabling function for the GPT (POEG).
>
> This patch series add basic support using s/w control through
> sysfs to enable/disable output from GPT.
>
> Signed-off-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> ---
> v1->v2:
> * Renamed the file poeg-rzg2l->rzg2l-poeg
> * Removed the macro POEGG as there is only single register and
> updated rzg2l_poeg_write() and rzg2l_poeg_read()
> * Updated error handling in probe()
Thanks for the update!
> --- /dev/null
> +++ b/drivers/pinctrl/renesas/poeg/Kconfig
> @@ -0,0 +1,12 @@
> +# SPDX-License-Identifier: GPL-2.0
> +config POEG_RZG2L
> + tristate "RZ/G2L poeg support"
> + depends on ARCH_RZG2L
> + depends on PWM_RZG2L_GPT
These are not hard dependencies, right?
And PWM_RZG2L_GPT (sort of) implies ARCH_RZG2L.
So I think the above two lines should be replaced by
depends on PWM_RZG2L_GPT || COMPILE_TEST
> + depends on HAS_IOMEM
> + help
> + This driver exposes the General Port Output Enable for PWM found
> + in RZ/G2L.
> +
> + To compile this driver as a module, choose M here: the module
> + will be called poeg-rzg2l.
> --- /dev/null
> +++ b/drivers/pinctrl/renesas/poeg/rzg2l-poeg.c
> @@ -0,0 +1,147 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Renesas RZ/G2L Port Output Enable for GPT (POEG) driver
> + *
> + * Copyright (C) 2022 Renesas Electronics Corporation
> + */
> +
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/reset.h>
> +
> +#define POEGG_SSF BIT(3)
> +
> +struct rzg2l_poeg_chip {
> + struct reset_control *rstc;
> + void __iomem *mmio;
> +};
> +
> +static void rzg2l_poeg_write(struct rzg2l_poeg_chip *chip, u32 data)
> +{
> + iowrite32(data, chip->mmio);
> +}
> +
> +static u32 rzg2l_poeg_read(struct rzg2l_poeg_chip *chip)
> +{
> + return ioread32(chip->mmio);
> +}
> +
> +static ssize_t output_disable_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct platform_device *pdev = to_platform_device(dev);
> + struct rzg2l_poeg_chip *chip = platform_get_drvdata(pdev);
chip = dev_get_drvdata(dev)
> + unsigned int val;
> + u32 reg_val;
> + int ret;
> +
> + ret = kstrtouint(buf, 0, &val);
> + if (ret)
> + return ret;
> +
> + reg_val = rzg2l_poeg_read(chip);
> + if (val)
> + reg_val |= POEGG_SSF;
> + else
> + reg_val &= ~POEGG_SSF;
> +
> + rzg2l_poeg_write(chip, reg_val);
> +
> + return ret ? : count;
> +}
> +
> +static ssize_t output_disable_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct platform_device *pdev = to_platform_device(dev);
> + struct rzg2l_poeg_chip *chip = platform_get_drvdata(pdev);
chip = dev_get_drvdata(dev)
> + u32 reg;
> +
> + reg = rzg2l_poeg_read(chip);
> +
> + return sprintf(buf, "%u\n", (reg & POEGG_SSF) ? 1 : 0);
sysfs_emit().
> +}
> +static DEVICE_ATTR_RW(output_disable);
Probably you want to document these properties under Documentation/,
or do you intend to have this as an interim solution?
TBH, I still don't know if this is the proper way to handle POEG,
but I see no better alternative...
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
