| Message ID | 20230921-ixp4xx-gpio-clocks-v1-2-574942bf944a@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | gpio: ixp4xx: Handle external clock output | expand |
On Thu, Sep 21, 2023 at 12:23:46AM +0200, Linus Walleij wrote: > This makes it possible to provide basic clock output on pins > 14 and 15. The clocks are typically used by random electronics, > not modeled in the device tree, so they just need to be provided > on request. ... > + val = __raw_readl(g->base + IXP4XX_REG_GPCLK); Do we need to read this... > /* > * Make sure GPIO 14 and 15 are NOT used as clocks but GPIO on > * specific machines. > */ > if (of_machine_is_compatible("dlink,dsm-g600-a") || > of_machine_is_compatible("iom,nas-100d")) > - __raw_writel(0x0, g->base + IXP4XX_REG_GPCLK); > + val = 0; ...if we are going to discard it anyway here? Maybe if (...) val = 0; else val = readl(); ? ... > + /* > + * Enable clock outputs with default timings of requested clock. > + * If you need control over TC and DC, add these to the device > + * tree bindings and use them here. > + */ Shouldn't this be integrated into PPS subsystem?
On Thu, Sep 21, 2023 at 1:23 PM Andy Shevchenko <andy@kernel.org> wrote: > Maybe > > if (...) > val = 0; > else > val = readl(); > > ? Neat, I'll do this! > > + /* > > + * Enable clock outputs with default timings of requested clock. > > + * If you need control over TC and DC, add these to the device > > + * tree bindings and use them here. > > + */ > > Shouldn't this be integrated into PPS subsystem? PPS is for GPS, we *could* integrate it into the clk subsystem (with configuration cells and all hoopla) but as I state in the cover letter I think it becomes overengineered for this legacy system, there are no users or benefit of that added complexity as compared to these few-liners. Yours, Linus Walleij
diff --git a/drivers/gpio/gpio-ixp4xx.c b/drivers/gpio/gpio-ixp4xx.c index dde6cf3a5779..6a9d32b4d0d7 100644 --- a/drivers/gpio/gpio-ixp4xx.c +++ b/drivers/gpio/gpio-ixp4xx.c @@ -38,6 +38,18 @@ #define IXP4XX_GPIO_STYLE_MASK GENMASK(2, 0) #define IXP4XX_GPIO_STYLE_SIZE 3 +/* + * Clock output control register defines. + */ +#define IXP4XX_GPCLK_CLK0DC_SHIFT 0 +#define IXP4XX_GPCLK_CLK0TC_SHIFT 4 +#define IXP4XX_GPCLK_CLK0_MASK GENMASK(7, 0) +#define IXP4XX_GPCLK_MUX14 BIT(8) +#define IXP4XX_GPCLK_CLK1DC_SHIFT 16 +#define IXP4XX_GPCLK_CLK1TC_SHIFT 20 +#define IXP4XX_GPCLK_CLK1_MASK GENMASK(23, 16) +#define IXP4XX_GPCLK_MUX15 BIT(24) + /** * struct ixp4xx_gpio - IXP4 GPIO state container * @dev: containing device for this instance @@ -202,6 +214,7 @@ static int ixp4xx_gpio_probe(struct platform_device *pdev) struct ixp4xx_gpio *g; struct gpio_irq_chip *girq; struct device_node *irq_parent; + u32 val; int ret; g = devm_kzalloc(dev, sizeof(*g), GFP_KERNEL); @@ -225,13 +238,34 @@ static int ixp4xx_gpio_probe(struct platform_device *pdev) } g->fwnode = of_node_to_fwnode(np); + val = __raw_readl(g->base + IXP4XX_REG_GPCLK); /* * Make sure GPIO 14 and 15 are NOT used as clocks but GPIO on * specific machines. */ if (of_machine_is_compatible("dlink,dsm-g600-a") || of_machine_is_compatible("iom,nas-100d")) - __raw_writel(0x0, g->base + IXP4XX_REG_GPCLK); + val = 0; + + /* + * Enable clock outputs with default timings of requested clock. + * If you need control over TC and DC, add these to the device + * tree bindings and use them here. + */ + if (of_property_read_bool(np, "intel,ixp4xx-gpio14-clkout")) { + val &= ~IXP4XX_GPCLK_CLK0_MASK; + val |= (0 << IXP4XX_GPCLK_CLK0DC_SHIFT); + val |= (1 << IXP4XX_GPCLK_CLK0TC_SHIFT); + val |= IXP4XX_GPCLK_MUX14; + } + + if (of_property_read_bool(np, "intel,ixp4xx-gpio15-clkout")) { + val &= ~IXP4XX_GPCLK_CLK1_MASK; + val |= (0 << IXP4XX_GPCLK_CLK1DC_SHIFT); + val |= (1 << IXP4XX_GPCLK_CLK1TC_SHIFT); + val |= IXP4XX_GPCLK_MUX15; + } + __raw_writel(val, g->base + IXP4XX_REG_GPCLK); /* * This is a very special big-endian ARM issue: when the IXP4xx is
This makes it possible to provide basic clock output on pins 14 and 15. The clocks are typically used by random electronics, not modeled in the device tree, so they just need to be provided on request. Signed-off-by: Linus Walleij <linus.walleij@linaro.org> --- drivers/gpio/gpio-ixp4xx.c | 36 +++++++++++++++++++++++++++++++++++- 1 file changed, 35 insertions(+), 1 deletion(-)