diff mbox series

[v3,07/42] soc: Add SoC driver for Cirrus ep93xx

Message ID 20230605-ep93xx-v3-7-3d63a5f1103e@maquefel.me (mailing list archive)
State Not Applicable
Headers show
Series ep93xx device tree conversion | expand

Checks

Context Check Description
netdev/series_format fail Series longer than 15 patches (and no cover letter)
netdev/tree_selection success Guessed tree name to be net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit fail Errors and warnings before: 17 this patch: 1352
netdev/cc_maintainers warning 6 maintainers not CCed: j.neuschaefer@gmx.net conor.dooley@microchip.com joel@jms.id.au walker.chen@starfivetech.com zhuyinbo@loongson.cn kernel@esmil.dk
netdev/build_clang fail Errors and warnings before: 22 this patch: 1373
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn fail Errors and warnings before: 17 this patch: 1375
netdev/checkpatch fail CHECK: Alignment should match open parenthesis ERROR: open brace '{' following function definitions go on the next line WARNING: added, moved or deleted file(s), does MAINTAINERS need updating? WARNING: line length of 81 exceeds 80 columns WARNING: line length of 85 exceeds 80 columns WARNING: line length of 86 exceeds 80 columns WARNING: line length of 89 exceeds 80 columns WARNING: line length of 90 exceeds 80 columns WARNING: line length of 96 exceeds 80 columns WARNING: line length of 97 exceeds 80 columns WARNING: please write a help paragraph that fully describes the config symbol
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Nikita Shubin via B4 Relay July 20, 2023, 11:29 a.m. UTC
From: Nikita Shubin <nikita.shubin@maquefel.me>

This adds an SoC driver for the ep93xx. Currently there
is only one thing not fitting into any other framework,
and that is the swlock setting.

It's used for clock settings and restart.

Signed-off-by: Nikita Shubin <nikita.shubin@maquefel.me>
Tested-by: Alexander Sverdlin <alexander.sverdlin@gmail.com>
Acked-by: Alexander Sverdlin <alexander.sverdlin@gmail.com>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/soc/Kconfig               |   1 +
 drivers/soc/Makefile              |   1 +
 drivers/soc/cirrus/Kconfig        |  12 ++
 drivers/soc/cirrus/Makefile       |   2 +
 drivers/soc/cirrus/soc-ep93xx.c   | 231 ++++++++++++++++++++++++++++++++++++++
 include/linux/soc/cirrus/ep93xx.h |  18 ++-
 6 files changed, 264 insertions(+), 1 deletion(-)

Comments

Andy Shevchenko July 21, 2023, 2:13 p.m. UTC | #1
On Thu, Jul 20, 2023 at 02:29:07PM +0300, Nikita Shubin via B4 Relay wrote:
> From: Nikita Shubin <nikita.shubin@maquefel.me>
> 
> This adds an SoC driver for the ep93xx. Currently there
> is only one thing not fitting into any other framework,
> and that is the swlock setting.
> 
> It's used for clock settings and restart.

> +#include <linux/clk-provider.h>
> +#include <linux/init.h>
> +#include <linux/kernel.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/of.h>
> +#include <linux/regmap.h>
> +#include <linux/slab.h>
> +#include <linux/sys_soc.h>
> +#include <linux/soc/cirrus/ep93xx.h>

...

> +#define EP93XX_SYSCON_SYSCFG_REV_MASK	0xf0000000

GENMASK() (you will need bits.h, and looking below you seem missed it anyway)

...

> +	spin_lock_irqsave(&ep93xx_swlock, flags);
> +
> +	regmap_read(map, EP93XX_SYSCON_DEVCFG, &val);
> +	val &= ~clear_bits;
> +	val |= set_bits;
> +	regmap_write(map, EP93XX_SYSCON_SWLOCK, EP93XX_SWLOCK_MAGICK);
> +	regmap_write(map, EP93XX_SYSCON_DEVCFG, val);

Is this sequence a must?
I.o.w. can you first supply magic and then update devcfg?

> +	spin_unlock_irqrestore(&ep93xx_swlock, flags);

...

> +void ep93xx_swlocked_update_bits(struct regmap *map, unsigned int reg,
> +				 unsigned int mask, unsigned int val)
> +{

Same Q as above.

> +}

...

> +	int rev = ep93xx_chip_revision(map);
> +
> +	switch (rev) {

	switch(ep93xx_chip_revision(map))

?

> +	case EP93XX_CHIP_REV_D0:
> +		return "D0";
> +	case EP93XX_CHIP_REV_D1:
> +		return "D1";
> +	case EP93XX_CHIP_REV_E0:
> +		return "E0";
> +	case EP93XX_CHIP_REV_E1:
> +		return "E1";
> +	case EP93XX_CHIP_REV_E2:
> +		return "E2";
> +	default:
> +		return "unknown";
> +	}

...

> +static unsigned long __init calc_pll_rate(u64 rate, u32 config_word)
> +{
> +	rate *= ((config_word >> 11) & GENMASK(4, 0)) + 1;	/* X1FBD */
> +	rate *= ((config_word >> 5) & GENMASK(5, 0)) + 1;	/* X2FBD */
> +	do_div(rate, (config_word & GENMASK(4, 0)) + 1);	/* X2IPD */
> +	rate >>= ((config_word >> 16) & 3);			/* PS */

GENMASK()

> +	return rate;
> +}

...

> +	/* Multiplatform guard, only proceed on ep93xx */
> +	if (!of_machine_is_compatible("cirrus,ep9301"))
> +		return 0;

Why?

> +	map = syscon_regmap_lookup_by_compatible("cirrus,ep9301-syscon");
> +	if (IS_ERR(map))
> +		return PTR_ERR(map);

Is this not enough?

...

> +	if (!(value & EP93XX_SYSCON_CLKSET1_NBYP1))
> +		clk_pll1_rate = EP93XX_EXT_CLK_RATE;
> +	else
> +		clk_pll1_rate = calc_pll_rate(EP93XX_EXT_CLK_RATE, value);

Positive conditionals in if-else are easier to be read.

...

> +	clk_f_div = fclk_divisors[(value >> 25) & 0x7];
> +	clk_h_div = hclk_divisors[(value >> 20) & 0x7];
> +	clk_p_div = pclk_divisors[(value >> 18) & 0x3];

GENMASK() in all three?

...

> +	np = of_find_node_by_path("/");
> +	of_property_read_string(np, "model", &machine);
> +	if (machine)
> +		attrs->machine = kstrdup(machine, GFP_KERNEL);

No error check?

> +	of_node_put(np);

Looks like NIH of_flat_dt_get_machine_name(). Can you rather make use of it as
it's done, e.g., here arch/riscv/kernel/setup.c:251?
Krzysztof Kozlowski July 21, 2023, 2:22 p.m. UTC | #2
On 20/07/2023 13:29, Nikita Shubin via B4 Relay wrote:
> From: Nikita Shubin <nikita.shubin@maquefel.me>
> 
> This adds an SoC driver for the ep93xx. Currently there
> is only one thing not fitting into any other framework,
> and that is the swlock setting.
> 
> It's used for clock settings and restart.
> 
> Signed-off-by: Nikita Shubin <nikita.shubin@maquefel.me>
> Tested-by: Alexander Sverdlin <alexander.sverdlin@gmail.com>
> Acked-by: Alexander Sverdlin <alexander.sverdlin@gmail.com>
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> ---
>  drivers/soc/Kconfig               |   1 +
>  drivers/soc/Makefile              |   1 +
>  drivers/soc/cirrus/Kconfig        |  12 ++
>  drivers/soc/cirrus/Makefile       |   2 +
>  drivers/soc/cirrus/soc-ep93xx.c   | 231 ++++++++++++++++++++++++++++++++++++++
>  include/linux/soc/cirrus/ep93xx.h |  18 ++-
>  6 files changed, 264 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/soc/Kconfig b/drivers/soc/Kconfig
> index 4e176280113a..16327b63b773 100644
> --- a/drivers/soc/Kconfig
> +++ b/drivers/soc/Kconfig
> @@ -8,6 +8,7 @@ source "drivers/soc/aspeed/Kconfig"
>  source "drivers/soc/atmel/Kconfig"
>  source "drivers/soc/bcm/Kconfig"
>  source "drivers/soc/canaan/Kconfig"
> +source "drivers/soc/cirrus/Kconfig"
>  source "drivers/soc/fsl/Kconfig"
>  source "drivers/soc/fujitsu/Kconfig"
>  source "drivers/soc/imx/Kconfig"
> diff --git a/drivers/soc/Makefile b/drivers/soc/Makefile
> index 3b0f9fb3b5c8..b76a03fe808e 100644
> --- a/drivers/soc/Makefile
> +++ b/drivers/soc/Makefile
> @@ -9,6 +9,7 @@ obj-y				+= aspeed/
>  obj-$(CONFIG_ARCH_AT91)		+= atmel/
>  obj-y				+= bcm/
>  obj-$(CONFIG_SOC_CANAAN)	+= canaan/
> +obj-$(CONFIG_EP93XX_SOC)        += cirrus/
>  obj-$(CONFIG_ARCH_DOVE)		+= dove/
>  obj-$(CONFIG_MACH_DOVE)		+= dove/
>  obj-y				+= fsl/
> diff --git a/drivers/soc/cirrus/Kconfig b/drivers/soc/cirrus/Kconfig
> new file mode 100644
> index 000000000000..408f3343a265
> --- /dev/null
> +++ b/drivers/soc/cirrus/Kconfig
> @@ -0,0 +1,12 @@
> +# SPDX-License-Identifier: GPL-2.0
> +
> +if ARCH_EP93XX
> +
> +config EP93XX_SOC
> +	bool "Cirrus EP93xx chips SoC"
> +	select SOC_BUS
> +	default y if !EP93XX_SOC_COMMON
> +	help
> +	  Support SoC for Cirrus EP93xx chips.
> +
> +endif
> diff --git a/drivers/soc/cirrus/Makefile b/drivers/soc/cirrus/Makefile
> new file mode 100644
> index 000000000000..ed6752844c6f
> --- /dev/null
> +++ b/drivers/soc/cirrus/Makefile
> @@ -0,0 +1,2 @@
> +# SPDX-License-Identifier: GPL-2.0
> +obj-y	+= soc-ep93xx.o
> diff --git a/drivers/soc/cirrus/soc-ep93xx.c b/drivers/soc/cirrus/soc-ep93xx.c
> new file mode 100644
> index 000000000000..2fd48d900f24
> --- /dev/null
> +++ b/drivers/soc/cirrus/soc-ep93xx.c
> @@ -0,0 +1,231 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * SoC driver for Cirrus EP93xx chips.
> + * Copyright (C) 2022 Nikita Shubin <nikita.shubin@maquefel.me>
> + *
> + * Based on a rewrite of arch/arm/mach-ep93xx/core.c
> + * Copyright (C) 2006 Lennert Buytenhek <buytenh@wantstofly.org>
> + * Copyright (C) 2007 Herbert Valerio Riedel <hvr@gnu.org>
> + *
> + * Thanks go to Michael Burian and Ray Lehtiniemi for their key
> + * role in the ep93xx Linux community
> + */
> +
> +#include <linux/clk-provider.h>
> +#include <linux/init.h>
> +#include <linux/kernel.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/of.h>
> +#include <linux/regmap.h>
> +#include <linux/slab.h>
> +#include <linux/sys_soc.h>
> +#include <linux/soc/cirrus/ep93xx.h>
> +
> +#define EP93XX_EXT_CLK_RATE		14745600
> +
> +#define EP93XX_SYSCON_DEVCFG		0x80
> +
> +#define EP93XX_SWLOCK_MAGICK		0xaa
> +#define EP93XX_SYSCON_SWLOCK		0xc0
> +#define EP93XX_SYSCON_SYSCFG		0x9c
> +#define EP93XX_SYSCON_SYSCFG_REV_MASK	0xf0000000
> +#define EP93XX_SYSCON_SYSCFG_REV_SHIFT	28
> +
> +#define EP93XX_SYSCON_CLKSET1		0x20
> +#define EP93XX_SYSCON_CLKSET1_NBYP1	BIT(23)
> +#define EP93XX_SYSCON_CLKSET2		0x24
> +#define EP93XX_SYSCON_CLKSET2_NBYP2	BIT(19)
> +#define EP93XX_SYSCON_CLKSET2_PLL2_EN	BIT(18)
> +
> +static DEFINE_SPINLOCK(ep93xx_swlock);
> +
> +/* EP93xx System Controller software locked register write */
> +void ep93xx_syscon_swlocked_write(struct regmap *map, unsigned int reg, unsigned int val)
> +{
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&ep93xx_swlock, flags);
> +
> +	regmap_write(map, EP93XX_SYSCON_SWLOCK, EP93XX_SWLOCK_MAGICK);
> +	regmap_write(map, reg, val);
> +
> +	spin_unlock_irqrestore(&ep93xx_swlock, flags);
> +}
> +EXPORT_SYMBOL_NS_GPL(ep93xx_syscon_swlocked_write, EP93XX_SOC);

I doubt that your code compiles. Didn't you add a user of this in some
earlier patch?

Anyway, no, drop it, don't export some weird calls from core initcall to
drivers. You violate layering and driver encapsulation. There is no
dependency/probe ordering.

There is no even need for this, because this code does not use it!

> +
> +void ep93xx_devcfg_set_clear(struct regmap *map, unsigned int set_bits, unsigned int clear_bits)
> +{
> +	unsigned long flags;
> +	unsigned int val;
> +
> +	spin_lock_irqsave(&ep93xx_swlock, flags);
> +
> +	regmap_read(map, EP93XX_SYSCON_DEVCFG, &val);
> +	val &= ~clear_bits;
> +	val |= set_bits;
> +	regmap_write(map, EP93XX_SYSCON_SWLOCK, EP93XX_SWLOCK_MAGICK);
> +	regmap_write(map, EP93XX_SYSCON_DEVCFG, val);
> +
> +	spin_unlock_irqrestore(&ep93xx_swlock, flags);
> +}
> +EXPORT_SYMBOL_NS_GPL(ep93xx_devcfg_set_clear, EP93XX_SOC);

No.

> +
> +void ep93xx_swlocked_update_bits(struct regmap *map, unsigned int reg,
> +				 unsigned int mask, unsigned int val)
> +{
> +	unsigned long flags;
> +	unsigned int tmp, orig;
> +
> +	spin_lock_irqsave(&ep93xx_swlock, flags);
> +
> +	regmap_read(map, EP93XX_SYSCON_DEVCFG, &orig);
> +	tmp = orig & ~mask;
> +	tmp |= val & mask;
> +	if (tmp != orig) {
> +		regmap_write(map, EP93XX_SYSCON_SWLOCK, EP93XX_SWLOCK_MAGICK);
> +		regmap_write(map, reg, tmp);
> +	}
> +
> +	spin_unlock_irqrestore(&ep93xx_swlock, flags);
> +}
> +EXPORT_SYMBOL_NS_GPL(ep93xx_swlocked_update_bits, EP93XX_SOC);

No.


> +
> +unsigned int __init ep93xx_chip_revision(struct regmap *map)

Why this is visible outside? This should be static.


> +{
> +	unsigned int val;
> +
> +	regmap_read(map, EP93XX_SYSCON_SYSCFG, &val);
> +	val &= EP93XX_SYSCON_SYSCFG_REV_MASK;
> +	val >>= EP93XX_SYSCON_SYSCFG_REV_SHIFT;
> +	return val;
> +}


> +
> +static const char __init *ep93xx_get_soc_rev(struct regmap *map)
> +{
> +	int rev = ep93xx_chip_revision(map);
> +
> +	switch (rev) {
> +	case EP93XX_CHIP_REV_D0:
> +		return "D0";
> +	case EP93XX_CHIP_REV_D1:
> +		return "D1";
> +	case EP93XX_CHIP_REV_E0:
> +		return "E0";
> +	case EP93XX_CHIP_REV_E1:
> +		return "E1";
> +	case EP93XX_CHIP_REV_E2:
> +		return "E2";
> +	default:
> +		return "unknown";
> +	}
> +}
> +
> +/*
> + * PLL rate = 14.7456 MHz * (X1FBD + 1) * (X2FBD + 1) / (X2IPD + 1) / 2^PS
> + */
> +static unsigned long __init calc_pll_rate(u64 rate, u32 config_word)
> +{
> +	rate *= ((config_word >> 11) & GENMASK(4, 0)) + 1;	/* X1FBD */
> +	rate *= ((config_word >> 5) & GENMASK(5, 0)) + 1;	/* X2FBD */
> +	do_div(rate, (config_word & GENMASK(4, 0)) + 1);	/* X2IPD */
> +	rate >>= ((config_word >> 16) & 3);			/* PS */
> +
> +	return rate;
> +}
> +
> +static int __init ep93xx_soc_init(void)
> +{
> +	struct soc_device_attribute *attrs;
> +	struct soc_device *soc_dev;
> +	struct device_node *np;
> +	struct regmap *map;
> +	struct clk_hw *hw;
> +	unsigned long clk_pll1_rate, clk_pll2_rate;
> +	unsigned int clk_f_div, clk_h_div, clk_p_div, clk_usb_div;
> +	const char fclk_divisors[] = { 1, 2, 4, 8, 16, 1, 1, 1 };
> +	const char hclk_divisors[] = { 1, 2, 4, 5, 6, 8, 16, 32 };
> +	const char pclk_divisors[] = { 1, 2, 4, 8 };
> +	const char *machine = NULL;
> +	u32 value;
> +
> +	/* Multiplatform guard, only proceed on ep93xx */
> +	if (!of_machine_is_compatible("cirrus,ep9301"))
> +		return 0;

This should already be a warning sign for you...

> +
> +	map = syscon_regmap_lookup_by_compatible("cirrus,ep9301-syscon");
> +	if (IS_ERR(map))
> +		return PTR_ERR(map);

No, not-reusable. Use devices and device nodes.

> +
> +	/* Determine the bootloader configured pll1 rate */
> +	regmap_read(map, EP93XX_SYSCON_CLKSET1, &value);
> +	if (!(value & EP93XX_SYSCON_CLKSET1_NBYP1))
> +		clk_pll1_rate = EP93XX_EXT_CLK_RATE;
> +	else
> +		clk_pll1_rate = calc_pll_rate(EP93XX_EXT_CLK_RATE, value);
> +
> +	hw = clk_hw_register_fixed_rate(NULL, "pll1", "xtali", 0, clk_pll1_rate);
> +	if (IS_ERR(hw))
> +		return PTR_ERR(hw);
> +
> +	/* Initialize the pll1 derived clocks */
> +	clk_f_div = fclk_divisors[(value >> 25) & 0x7];
> +	clk_h_div = hclk_divisors[(value >> 20) & 0x7];
> +	clk_p_div = pclk_divisors[(value >> 18) & 0x3];
> +
> +	hw = clk_hw_register_fixed_factor(NULL, "fclk", "pll1", 0, 1, clk_f_div);
> +	if (IS_ERR(hw))
> +		return PTR_ERR(hw);
> +
> +	hw = clk_hw_register_fixed_factor(NULL, "hclk", "pll1", 0, 1, clk_h_div);
> +	if (IS_ERR(hw))
> +		return PTR_ERR(hw);
> +
> +	hw = clk_hw_register_fixed_factor(NULL, "pclk", "hclk", 0, 1, clk_p_div);
> +	if (IS_ERR(hw))
> +		return PTR_ERR(hw);
> +
> +	/* Determine the bootloader configured pll2 rate */
> +	regmap_read(map, EP93XX_SYSCON_CLKSET2, &value);
> +	if (!(value & EP93XX_SYSCON_CLKSET2_NBYP2))
> +		clk_pll2_rate = EP93XX_EXT_CLK_RATE;
> +	else if (value & EP93XX_SYSCON_CLKSET2_PLL2_EN)
> +		clk_pll2_rate = calc_pll_rate(EP93XX_EXT_CLK_RATE, value);
> +	else
> +		clk_pll2_rate = 0;
> +
> +	hw = clk_hw_register_fixed_rate(NULL, "pll2", "xtali", 0, clk_pll2_rate);
> +	if (IS_ERR(hw))
> +		return PTR_ERR(hw);
> +
> +	regmap_read(map, EP93XX_SYSCON_CLKSET2, &value);
> +	clk_usb_div = (((value >> 28) & GENMASK(3, 0)) + 1);
> +	hw = clk_hw_register_fixed_factor(NULL, "usb_clk", "pll2", 0, 1, clk_usb_div);
> +	if (IS_ERR(hw))
> +		return PTR_ERR(hw);
> +
> +	attrs = kzalloc(sizeof(*attrs), GFP_KERNEL);
> +	if (!attrs)
> +		return -ENOMEM;
> +
> +	np = of_find_node_by_path("/");
> +	of_property_read_string(np, "model", &machine);
> +	if (machine)
> +		attrs->machine = kstrdup(machine, GFP_KERNEL);
> +	of_node_put(np);
> +
> +	attrs->family = "Cirrus Logic EP93xx";
> +	attrs->revision = ep93xx_get_soc_rev(map);
> +
> +	soc_dev = soc_device_register(attrs);
> +	if (IS_ERR(soc_dev)) {
> +		kfree(attrs->soc_id);
> +		kfree(attrs->serial_number);
> +		kfree(attrs);
> +		return PTR_ERR(soc_dev);
> +	}
> +
> +	pr_info("EP93xx SoC revision %s\n", attrs->revision);
> +
> +	return 0;
> +}
> +core_initcall(ep93xx_soc_init);

That's not the way to add soc driver. You need a proper driver for it

Best regards,
Krzysztof
Alexander Sverdlin Nov. 11, 2023, 9:33 p.m. UTC | #3
Hello Andy,

On Fri, 2023-07-21 at 17:13 +0300, Andy Shevchenko wrote:
> > +       spin_lock_irqsave(&ep93xx_swlock, flags);
> > +
> > +       regmap_read(map, EP93XX_SYSCON_DEVCFG, &val);
> > +       val &= ~clear_bits;
> > +       val |= set_bits;
> > +       regmap_write(map, EP93XX_SYSCON_SWLOCK, EP93XX_SWLOCK_MAGICK);
> > +       regmap_write(map, EP93XX_SYSCON_DEVCFG, val);
> 
> Is this sequence a must?
> I.o.w. can you first supply magic and then update devcfg?
> 
> > +       spin_unlock_irqrestore(&ep93xx_swlock, flags);
> 
> ...
> 
> > +void ep93xx_swlocked_update_bits(struct regmap *map, unsigned int reg,
> > +                                unsigned int mask, unsigned int val)
> > +{
> 
> Same Q as above.

EP93xx User Manual [1] has most verbose description of SWLock for ADC
block:
"Writing 0xAA to this register will unlock all locked registers until the
next block access. The ARM lock instruction prefix should be used for the
two consequtive write cycles when writing to locked chip registers."

One may conclude that RmW (two accesses to the particular block) sequence
is not appropriate.

[1] https://cdn.embeddedts.com/resource-attachments/ts-7000_ep9301-ug.pdf
Andy Shevchenko Nov. 13, 2023, 10:19 a.m. UTC | #4
On Sat, Nov 11, 2023 at 11:33 PM Alexander Sverdlin
<alexander.sverdlin@gmail.com> wrote:
> On Fri, 2023-07-21 at 17:13 +0300, Andy Shevchenko wrote:

...

> > > +       spin_lock_irqsave(&ep93xx_swlock, flags);
> > > +
> > > +       regmap_read(map, EP93XX_SYSCON_DEVCFG, &val);
> > > +       val &= ~clear_bits;
> > > +       val |= set_bits;
> > > +       regmap_write(map, EP93XX_SYSCON_SWLOCK, EP93XX_SWLOCK_MAGICK);
> > > +       regmap_write(map, EP93XX_SYSCON_DEVCFG, val);
> >
> > Is this sequence a must?
> > I.o.w. can you first supply magic and then update devcfg?
> >
> > > +       spin_unlock_irqrestore(&ep93xx_swlock, flags);

...

> > > +void ep93xx_swlocked_update_bits(struct regmap *map, unsigned int reg,
> > > +                                unsigned int mask, unsigned int val)

> > Same Q as above.
>
> EP93xx User Manual [1] has most verbose description of SWLock for ADC
> block:
> "Writing 0xAA to this register will unlock all locked registers until the
> next block access. The ARM lock instruction prefix should be used for the
> two consequtive write cycles when writing to locked chip registers."
>
> One may conclude that RmW (two accesses to the particular block) sequence
> is not appropriate.

It's not obvious to me. The terms "block access" and "block cycle"
occur only once in the very same section that describes ADCSWLock. The
meaning of these terms is not fully understandable. So, assuming that
you have tried it differently and it indeed doesn't work, let's go
with this one.

> [1] https://cdn.embeddedts.com/resource-attachments/ts-7000_ep9301-ug.pdf



--
With Best Regards,
Andy Shevchenko
diff mbox series

Patch

diff --git a/drivers/soc/Kconfig b/drivers/soc/Kconfig
index 4e176280113a..16327b63b773 100644
--- a/drivers/soc/Kconfig
+++ b/drivers/soc/Kconfig
@@ -8,6 +8,7 @@  source "drivers/soc/aspeed/Kconfig"
 source "drivers/soc/atmel/Kconfig"
 source "drivers/soc/bcm/Kconfig"
 source "drivers/soc/canaan/Kconfig"
+source "drivers/soc/cirrus/Kconfig"
 source "drivers/soc/fsl/Kconfig"
 source "drivers/soc/fujitsu/Kconfig"
 source "drivers/soc/imx/Kconfig"
diff --git a/drivers/soc/Makefile b/drivers/soc/Makefile
index 3b0f9fb3b5c8..b76a03fe808e 100644
--- a/drivers/soc/Makefile
+++ b/drivers/soc/Makefile
@@ -9,6 +9,7 @@  obj-y				+= aspeed/
 obj-$(CONFIG_ARCH_AT91)		+= atmel/
 obj-y				+= bcm/
 obj-$(CONFIG_SOC_CANAAN)	+= canaan/
+obj-$(CONFIG_EP93XX_SOC)        += cirrus/
 obj-$(CONFIG_ARCH_DOVE)		+= dove/
 obj-$(CONFIG_MACH_DOVE)		+= dove/
 obj-y				+= fsl/
diff --git a/drivers/soc/cirrus/Kconfig b/drivers/soc/cirrus/Kconfig
new file mode 100644
index 000000000000..408f3343a265
--- /dev/null
+++ b/drivers/soc/cirrus/Kconfig
@@ -0,0 +1,12 @@ 
+# SPDX-License-Identifier: GPL-2.0
+
+if ARCH_EP93XX
+
+config EP93XX_SOC
+	bool "Cirrus EP93xx chips SoC"
+	select SOC_BUS
+	default y if !EP93XX_SOC_COMMON
+	help
+	  Support SoC for Cirrus EP93xx chips.
+
+endif
diff --git a/drivers/soc/cirrus/Makefile b/drivers/soc/cirrus/Makefile
new file mode 100644
index 000000000000..ed6752844c6f
--- /dev/null
+++ b/drivers/soc/cirrus/Makefile
@@ -0,0 +1,2 @@ 
+# SPDX-License-Identifier: GPL-2.0
+obj-y	+= soc-ep93xx.o
diff --git a/drivers/soc/cirrus/soc-ep93xx.c b/drivers/soc/cirrus/soc-ep93xx.c
new file mode 100644
index 000000000000..2fd48d900f24
--- /dev/null
+++ b/drivers/soc/cirrus/soc-ep93xx.c
@@ -0,0 +1,231 @@ 
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * SoC driver for Cirrus EP93xx chips.
+ * Copyright (C) 2022 Nikita Shubin <nikita.shubin@maquefel.me>
+ *
+ * Based on a rewrite of arch/arm/mach-ep93xx/core.c
+ * Copyright (C) 2006 Lennert Buytenhek <buytenh@wantstofly.org>
+ * Copyright (C) 2007 Herbert Valerio Riedel <hvr@gnu.org>
+ *
+ * Thanks go to Michael Burian and Ray Lehtiniemi for their key
+ * role in the ep93xx Linux community
+ */
+
+#include <linux/clk-provider.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/mfd/syscon.h>
+#include <linux/of.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+#include <linux/sys_soc.h>
+#include <linux/soc/cirrus/ep93xx.h>
+
+#define EP93XX_EXT_CLK_RATE		14745600
+
+#define EP93XX_SYSCON_DEVCFG		0x80
+
+#define EP93XX_SWLOCK_MAGICK		0xaa
+#define EP93XX_SYSCON_SWLOCK		0xc0
+#define EP93XX_SYSCON_SYSCFG		0x9c
+#define EP93XX_SYSCON_SYSCFG_REV_MASK	0xf0000000
+#define EP93XX_SYSCON_SYSCFG_REV_SHIFT	28
+
+#define EP93XX_SYSCON_CLKSET1		0x20
+#define EP93XX_SYSCON_CLKSET1_NBYP1	BIT(23)
+#define EP93XX_SYSCON_CLKSET2		0x24
+#define EP93XX_SYSCON_CLKSET2_NBYP2	BIT(19)
+#define EP93XX_SYSCON_CLKSET2_PLL2_EN	BIT(18)
+
+static DEFINE_SPINLOCK(ep93xx_swlock);
+
+/* EP93xx System Controller software locked register write */
+void ep93xx_syscon_swlocked_write(struct regmap *map, unsigned int reg, unsigned int val)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&ep93xx_swlock, flags);
+
+	regmap_write(map, EP93XX_SYSCON_SWLOCK, EP93XX_SWLOCK_MAGICK);
+	regmap_write(map, reg, val);
+
+	spin_unlock_irqrestore(&ep93xx_swlock, flags);
+}
+EXPORT_SYMBOL_NS_GPL(ep93xx_syscon_swlocked_write, EP93XX_SOC);
+
+void ep93xx_devcfg_set_clear(struct regmap *map, unsigned int set_bits, unsigned int clear_bits)
+{
+	unsigned long flags;
+	unsigned int val;
+
+	spin_lock_irqsave(&ep93xx_swlock, flags);
+
+	regmap_read(map, EP93XX_SYSCON_DEVCFG, &val);
+	val &= ~clear_bits;
+	val |= set_bits;
+	regmap_write(map, EP93XX_SYSCON_SWLOCK, EP93XX_SWLOCK_MAGICK);
+	regmap_write(map, EP93XX_SYSCON_DEVCFG, val);
+
+	spin_unlock_irqrestore(&ep93xx_swlock, flags);
+}
+EXPORT_SYMBOL_NS_GPL(ep93xx_devcfg_set_clear, EP93XX_SOC);
+
+void ep93xx_swlocked_update_bits(struct regmap *map, unsigned int reg,
+				 unsigned int mask, unsigned int val)
+{
+	unsigned long flags;
+	unsigned int tmp, orig;
+
+	spin_lock_irqsave(&ep93xx_swlock, flags);
+
+	regmap_read(map, EP93XX_SYSCON_DEVCFG, &orig);
+	tmp = orig & ~mask;
+	tmp |= val & mask;
+	if (tmp != orig) {
+		regmap_write(map, EP93XX_SYSCON_SWLOCK, EP93XX_SWLOCK_MAGICK);
+		regmap_write(map, reg, tmp);
+	}
+
+	spin_unlock_irqrestore(&ep93xx_swlock, flags);
+}
+EXPORT_SYMBOL_NS_GPL(ep93xx_swlocked_update_bits, EP93XX_SOC);
+
+unsigned int __init ep93xx_chip_revision(struct regmap *map)
+{
+	unsigned int val;
+
+	regmap_read(map, EP93XX_SYSCON_SYSCFG, &val);
+	val &= EP93XX_SYSCON_SYSCFG_REV_MASK;
+	val >>= EP93XX_SYSCON_SYSCFG_REV_SHIFT;
+	return val;
+}
+
+static const char __init *ep93xx_get_soc_rev(struct regmap *map)
+{
+	int rev = ep93xx_chip_revision(map);
+
+	switch (rev) {
+	case EP93XX_CHIP_REV_D0:
+		return "D0";
+	case EP93XX_CHIP_REV_D1:
+		return "D1";
+	case EP93XX_CHIP_REV_E0:
+		return "E0";
+	case EP93XX_CHIP_REV_E1:
+		return "E1";
+	case EP93XX_CHIP_REV_E2:
+		return "E2";
+	default:
+		return "unknown";
+	}
+}
+
+/*
+ * PLL rate = 14.7456 MHz * (X1FBD + 1) * (X2FBD + 1) / (X2IPD + 1) / 2^PS
+ */
+static unsigned long __init calc_pll_rate(u64 rate, u32 config_word)
+{
+	rate *= ((config_word >> 11) & GENMASK(4, 0)) + 1;	/* X1FBD */
+	rate *= ((config_word >> 5) & GENMASK(5, 0)) + 1;	/* X2FBD */
+	do_div(rate, (config_word & GENMASK(4, 0)) + 1);	/* X2IPD */
+	rate >>= ((config_word >> 16) & 3);			/* PS */
+
+	return rate;
+}
+
+static int __init ep93xx_soc_init(void)
+{
+	struct soc_device_attribute *attrs;
+	struct soc_device *soc_dev;
+	struct device_node *np;
+	struct regmap *map;
+	struct clk_hw *hw;
+	unsigned long clk_pll1_rate, clk_pll2_rate;
+	unsigned int clk_f_div, clk_h_div, clk_p_div, clk_usb_div;
+	const char fclk_divisors[] = { 1, 2, 4, 8, 16, 1, 1, 1 };
+	const char hclk_divisors[] = { 1, 2, 4, 5, 6, 8, 16, 32 };
+	const char pclk_divisors[] = { 1, 2, 4, 8 };
+	const char *machine = NULL;
+	u32 value;
+
+	/* Multiplatform guard, only proceed on ep93xx */
+	if (!of_machine_is_compatible("cirrus,ep9301"))
+		return 0;
+
+	map = syscon_regmap_lookup_by_compatible("cirrus,ep9301-syscon");
+	if (IS_ERR(map))
+		return PTR_ERR(map);
+
+	/* Determine the bootloader configured pll1 rate */
+	regmap_read(map, EP93XX_SYSCON_CLKSET1, &value);
+	if (!(value & EP93XX_SYSCON_CLKSET1_NBYP1))
+		clk_pll1_rate = EP93XX_EXT_CLK_RATE;
+	else
+		clk_pll1_rate = calc_pll_rate(EP93XX_EXT_CLK_RATE, value);
+
+	hw = clk_hw_register_fixed_rate(NULL, "pll1", "xtali", 0, clk_pll1_rate);
+	if (IS_ERR(hw))
+		return PTR_ERR(hw);
+
+	/* Initialize the pll1 derived clocks */
+	clk_f_div = fclk_divisors[(value >> 25) & 0x7];
+	clk_h_div = hclk_divisors[(value >> 20) & 0x7];
+	clk_p_div = pclk_divisors[(value >> 18) & 0x3];
+
+	hw = clk_hw_register_fixed_factor(NULL, "fclk", "pll1", 0, 1, clk_f_div);
+	if (IS_ERR(hw))
+		return PTR_ERR(hw);
+
+	hw = clk_hw_register_fixed_factor(NULL, "hclk", "pll1", 0, 1, clk_h_div);
+	if (IS_ERR(hw))
+		return PTR_ERR(hw);
+
+	hw = clk_hw_register_fixed_factor(NULL, "pclk", "hclk", 0, 1, clk_p_div);
+	if (IS_ERR(hw))
+		return PTR_ERR(hw);
+
+	/* Determine the bootloader configured pll2 rate */
+	regmap_read(map, EP93XX_SYSCON_CLKSET2, &value);
+	if (!(value & EP93XX_SYSCON_CLKSET2_NBYP2))
+		clk_pll2_rate = EP93XX_EXT_CLK_RATE;
+	else if (value & EP93XX_SYSCON_CLKSET2_PLL2_EN)
+		clk_pll2_rate = calc_pll_rate(EP93XX_EXT_CLK_RATE, value);
+	else
+		clk_pll2_rate = 0;
+
+	hw = clk_hw_register_fixed_rate(NULL, "pll2", "xtali", 0, clk_pll2_rate);
+	if (IS_ERR(hw))
+		return PTR_ERR(hw);
+
+	regmap_read(map, EP93XX_SYSCON_CLKSET2, &value);
+	clk_usb_div = (((value >> 28) & GENMASK(3, 0)) + 1);
+	hw = clk_hw_register_fixed_factor(NULL, "usb_clk", "pll2", 0, 1, clk_usb_div);
+	if (IS_ERR(hw))
+		return PTR_ERR(hw);
+
+	attrs = kzalloc(sizeof(*attrs), GFP_KERNEL);
+	if (!attrs)
+		return -ENOMEM;
+
+	np = of_find_node_by_path("/");
+	of_property_read_string(np, "model", &machine);
+	if (machine)
+		attrs->machine = kstrdup(machine, GFP_KERNEL);
+	of_node_put(np);
+
+	attrs->family = "Cirrus Logic EP93xx";
+	attrs->revision = ep93xx_get_soc_rev(map);
+
+	soc_dev = soc_device_register(attrs);
+	if (IS_ERR(soc_dev)) {
+		kfree(attrs->soc_id);
+		kfree(attrs->serial_number);
+		kfree(attrs);
+		return PTR_ERR(soc_dev);
+	}
+
+	pr_info("EP93xx SoC revision %s\n", attrs->revision);
+
+	return 0;
+}
+core_initcall(ep93xx_soc_init);
diff --git a/include/linux/soc/cirrus/ep93xx.h b/include/linux/soc/cirrus/ep93xx.h
index 56fbe2dc59b1..f38937a6f08c 100644
--- a/include/linux/soc/cirrus/ep93xx.h
+++ b/include/linux/soc/cirrus/ep93xx.h
@@ -3,6 +3,7 @@ 
 #define _SOC_EP93XX_H
 
 struct platform_device;
+struct regmap;
 
 #define EP93XX_CHIP_REV_D0	3
 #define EP93XX_CHIP_REV_D1	4
@@ -10,7 +11,7 @@  struct platform_device;
 #define EP93XX_CHIP_REV_E1	6
 #define EP93XX_CHIP_REV_E2	7
 
-#ifdef CONFIG_ARCH_EP93XX
+#if defined(CONFIG_EP93XX_SOC_COMMON)
 int ep93xx_pwm_acquire_gpio(struct platform_device *pdev);
 void ep93xx_pwm_release_gpio(struct platform_device *pdev);
 int ep93xx_ide_acquire_gpio(struct platform_device *pdev);
@@ -30,7 +31,22 @@  static inline int ep93xx_keypad_acquire_gpio(struct platform_device *pdev) { ret
 static inline void ep93xx_keypad_release_gpio(struct platform_device *pdev) {}
 static inline int ep93xx_i2s_acquire(void) { return 0; }
 static inline void ep93xx_i2s_release(void) {}
+
+#if defined(CONFIG_EP93XX_SOC)
+void ep93xx_syscon_swlocked_write(struct regmap *map, unsigned int reg, unsigned int val);
+void ep93xx_devcfg_set_clear(struct regmap *map, unsigned int set_bits, unsigned int clear_bits);
+void ep93xx_swlocked_update_bits(struct regmap *map, unsigned int reg,
+				 unsigned int mask, unsigned int val);
+#else
+static inline void ep93xx_syscon_swlocked_write(struct regmap *map, unsigned int reg,
+						unsigned int val) { }
+static inline void ep93xx_devcfg_set_clear(struct regmap *map, unsigned int set_bits,
+						unsigned int clear_bits) { }
+
+void ep93xx_swlocked_update_bits(struct regmap *map, unsigned int reg,
+				unsigned int mask, unsigned int val) { }
 static inline unsigned int ep93xx_chip_revision(void) { return 0; }
+#endif
 
 #endif