* [PATCH] clk: imx51: uart4, uart5 gates only exist on imx50, imx53
From: Stephen Boyd @ 2017-12-22 0:00 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171213115748.14106-1-p.zabel@pengutronix.de>
On 12/13, Philipp Zabel wrote:
> i.MX51 only has 3 UARTs and no CCGR7 register. In place of the CCGR7
> register on i.MX50/i.MX53 that contains the ipg and per clock gates
> for UARTs 4 and 5, on i.MX51 there is the CMEOR register.
>
> Without this patch, the code disabling the UART clocks would also clear
> the mod_en_ov_vpu bit in the CMEOR register, among others, which causes
> register accesses to the VPU to lock up the system.
>
> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> ---
Applied to clk-next
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply
* [GIT PULL 1/2] Broadcom devicetree changes for 4.16
From: Florian Fainelli @ 2017-12-22 0:12 UTC (permalink / raw)
To: linux-arm-kernel
The following changes since commit 4fbd8d194f06c8a3fd2af1ce560ddb31f7ec8323:
Linux 4.15-rc1 (2017-11-26 16:01:47 -0800)
are available in the git repository at:
http://github.com/Broadcom/stblinux.git tags/arm-soc/for-4.16/devicetree
for you to fetch changes up to ececb5639c33a6a0444bd8e1cda8bf2ef20c6a6b:
Merge tag 'bcm2835-dt-next-2017-12-19' into devicetree/next (2017-12-20 17:32:58 -0800)
----------------------------------------------------------------
This pull request contains Broadcom ARM-based SoCs Device Tree changes for
4.16, please pull the following:
- Stefan updates the BCM283x DTS to make consistent use of the existing GPIO
defines for the polarity specifier
----------------------------------------------------------------
Florian Fainelli (1):
Merge tag 'bcm2835-dt-next-2017-12-19' into devicetree/next
Stefan Wahren (1):
ARM: dts: bcm283x: Use GPIO polarity defines consistently
arch/arm/boot/dts/bcm2835-rpi-a-plus.dts | 4 ++--
arch/arm/boot/dts/bcm2835-rpi-a.dts | 2 +-
arch/arm/boot/dts/bcm2835-rpi-b-plus.dts | 4 ++--
arch/arm/boot/dts/bcm2835-rpi-b-rev2.dts | 2 +-
arch/arm/boot/dts/bcm2835-rpi-b.dts | 2 +-
arch/arm/boot/dts/bcm2836-rpi-2-b.dts | 4 ++--
arch/arm/boot/dts/bcm2837-rpi-3-b.dts | 2 +-
7 files changed, 10 insertions(+), 10 deletions(-)
^ permalink raw reply
* [GIT PULL 2/2] Broadcom drivers changes for 4.16
From: Florian Fainelli @ 2017-12-22 0:12 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171222001227.30625-1-f.fainelli@gmail.com>
The following changes since commit 4fbd8d194f06c8a3fd2af1ce560ddb31f7ec8323:
Linux 4.15-rc1 (2017-11-26 16:01:47 -0800)
are available in the git repository at:
http://github.com/Broadcom/stblinux.git tags/arm-soc/for-4.16/drivers
for you to fetch changes up to f780429adfbc222a4d8a227a2a550ba627c7338b:
soc: brcmstb: biuctrl: Move to early_initcall (2017-12-20 17:37:44 -0800)
----------------------------------------------------------------
This pull request contains Broadcom ARM/ARM64 based SoCs drivers changes for
4.16, please pull the following:
- Arnd provides an update to the Raspberry Pi firmware interface and uses time64_t to
print the time to make it more future proof
- Florian provides a set of updates to make the Broadcom STB Bus Interface Unit code
work on newer ARM64-based chips, as well as perform the correct interface tuning
for these chips to reach the expected performance
----------------------------------------------------------------
Arnd Bergmann (1):
firmware: raspberrypi: print time using time64_t
Florian Fainelli (10):
Merge tag 'bcm2835-drivers-next-2017-12-19' into drivers/next
dt-bindings: arm: Add entry for Broadcom Brahma-B53
dt-bindings: arm: brcmstb: Correct BIUCTRL node documentation
soc: brcmstb: Make CPU credit offset more parameterized
soc: brcmstb: Correct CPU_CREDIT_REG offset for Brahma-B53 CPUs
soc: brcmstb: biuctrl: Prepare for saving/restoring other registers
soc: brcmstb: biuctrl: Wire-up new registers
soc: brcmstb: biuctrl: Fine tune B53 MCP interface settings
soc: brcmstb: Split initialization
soc: brcmstb: biuctrl: Move to early_initcall
.../devicetree/bindings/arm/bcm/brcm,brcmstb.txt | 22 +--
Documentation/devicetree/bindings/arm/cpus.txt | 1 +
arch/arm/mach-bcm/brcmstb.c | 2 -
drivers/firmware/raspberrypi.c | 2 +-
drivers/soc/bcm/brcmstb/biuctrl.c | 176 +++++++++++++++++++--
drivers/soc/bcm/brcmstb/common.c | 27 ++--
include/linux/soc/brcmstb/brcmstb.h | 6 -
7 files changed, 187 insertions(+), 49 deletions(-)
^ permalink raw reply
* [PATCH 4/4] ARM: dts: vf610-zii-dev-rev-b: add interrupts for 88e1545 PHY
From: Russell King - ARM Linux @ 2017-12-22 0:14 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CACRpkdarS+tPv5CG4bmFcPvc+=SJJcC1pbO7WSbsS5+yCuxh_Q@mail.gmail.com>
On Thu, Dec 21, 2017 at 11:53:47PM +0100, Linus Walleij wrote:
> On Thu, Dec 21, 2017 at 6:32 PM, Russell King - ARM Linux
> <linux@armlinux.org.uk> wrote:
>
> > What we have here is _really_ a shared interrupt between four
> > separate devices, and we need a way to sanely describe resources
> > shared between several device instances to pinmux. Unfortunately,
> > it seems pinmux is designed around one device having exclusive use
> > of a resource, which makes it hard to describe shared interrupts in
> > DT.
> >
> > Given that DT should be a description of the hardware, and should be
> > independent of the OS implementation, I'd say this is a pinmux bug,
> > because pinmux gets in the way of describing the hardware correctly.
> > ;)
>
> Hm that would be annoying. But when I look at it I think it would
> actually work. Did you try just assigning the same pin control
> state to all the PHY's and see what happens?
>
> Just set
> pinctrl-names = "default";
> pinctrl-0 = <&pinctrl_mv88e1545>;
>
> on all of them?
It was tried, DT was happy, but the kernel on boot complained because
pinctrl objected, which caused the drivers to fail to bind:
libphy: mdio: probed
vf610-pinctrl 40048000.iomuxc: pin VF610_PAD_PTB0 already requested by !mdio-mux!mdio at 4!switch at 0!mdio:00; cannot claim for !mdio-mux!mdio at 4!switch at 0!mdio:01
vf610-pinctrl 40048000.iomuxc: pin-22 (!mdio-mux!mdio at 4!switch at 0!mdio:01) status -22
vf610-pinctrl 40048000.iomuxc: could not request pin 22 (VF610_PAD_PTB0) from group pinctrl-mv88e1545 on device 40048000.iomuxc
Marvell 88E1545 !mdio-mux!mdio at 4!switch at 0!mdio:01: Error applying setting, reverse things back
Marvell 88E1545: probe of !mdio-mux!mdio at 4!switch at 0!mdio:01 failed with error -22
vf610-pinctrl 40048000.iomuxc: pin VF610_PAD_PTB0 already requested by !mdio-mux!mdio at 4!switch at 0!mdio:00; cannot claim for !mdio-mux!mdio at 4!switch at 0!mdio:02
vf610-pinctrl 40048000.iomuxc: pin-22 (!mdio-mux!mdio at 4!switch at 0!mdio:02) status -22
vf610-pinctrl 40048000.iomuxc: could not request pin 22 (VF610_PAD_PTB0) from group pinctrl-mv88e1545 on device 40048000.iomuxc
Marvell 88E1545 !mdio-mux!mdio at 4!switch at 0!mdio:02: Error applying setting, reverse things back
Marvell 88E1545: probe of !mdio-mux!mdio at 4!switch at 0!mdio:02 failed with error -22
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up
^ permalink raw reply
* [PATCH 4/4] ARM: dts: vf610-zii-dev-rev-b: add interrupts for 88e1545 PHY
From: Florian Fainelli @ 2017-12-22 0:20 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171222001407.GL10595@n2100.armlinux.org.uk>
On 12/21/2017 04:14 PM, Russell King - ARM Linux wrote:
> On Thu, Dec 21, 2017 at 11:53:47PM +0100, Linus Walleij wrote:
>> On Thu, Dec 21, 2017 at 6:32 PM, Russell King - ARM Linux
>> <linux@armlinux.org.uk> wrote:
>>
>>> What we have here is _really_ a shared interrupt between four
>>> separate devices, and we need a way to sanely describe resources
>>> shared between several device instances to pinmux. Unfortunately,
>>> it seems pinmux is designed around one device having exclusive use
>>> of a resource, which makes it hard to describe shared interrupts in
>>> DT.
>>>
>>> Given that DT should be a description of the hardware, and should be
>>> independent of the OS implementation, I'd say this is a pinmux bug,
>>> because pinmux gets in the way of describing the hardware correctly.
>>> ;)
>>
>> Hm that would be annoying. But when I look at it I think it would
>> actually work. Did you try just assigning the same pin control
>> state to all the PHY's and see what happens?
>>
>> Just set
>> pinctrl-names = "default";
>> pinctrl-0 = <&pinctrl_mv88e1545>;
>>
>> on all of them?
>
> It was tried, DT was happy, but the kernel on boot complained because
> pinctrl objected, which caused the drivers to fail to bind:
>
> libphy: mdio: probed
> vf610-pinctrl 40048000.iomuxc: pin VF610_PAD_PTB0 already requested by !mdio-mux!mdio at 4!switch at 0!mdio:00; cannot claim for !mdio-mux!mdio at 4!switch at 0!mdio:01
> vf610-pinctrl 40048000.iomuxc: pin-22 (!mdio-mux!mdio at 4!switch at 0!mdio:01) status -22
> vf610-pinctrl 40048000.iomuxc: could not request pin 22 (VF610_PAD_PTB0) from group pinctrl-mv88e1545 on device 40048000.iomuxc
> Marvell 88E1545 !mdio-mux!mdio at 4!switch at 0!mdio:01: Error applying setting, reverse things back
> Marvell 88E1545: probe of !mdio-mux!mdio at 4!switch at 0!mdio:01 failed with error -22
> vf610-pinctrl 40048000.iomuxc: pin VF610_PAD_PTB0 already requested by !mdio-mux!mdio at 4!switch at 0!mdio:00; cannot claim for !mdio-mux!mdio at 4!switch at 0!mdio:02
> vf610-pinctrl 40048000.iomuxc: pin-22 (!mdio-mux!mdio at 4!switch at 0!mdio:02) status -22
> vf610-pinctrl 40048000.iomuxc: could not request pin 22 (VF610_PAD_PTB0) from group pinctrl-mv88e1545 on device 40048000.iomuxc
> Marvell 88E1545 !mdio-mux!mdio at 4!switch at 0!mdio:02: Error applying setting, reverse things back
> Marvell 88E1545: probe of !mdio-mux!mdio at 4!switch at 0!mdio:02 failed with error -22
>
You could also see it another way, because this is a quad PHY in a
single package, you could theoretically have a representation that
exposes a node container for the 4 PHYs, and that container node
requests the pinmux/pinctrl. Of course, this would not work with the
MDIO code which would not go one level down, and would expect the PHYs
to be at the same level as the container node...
Oh well.
--
Florian
^ permalink raw reply
* [PATCH v2 2/2] PCI: mediatek: Fixup class type for MT7622
From: Bjorn Helgaas @ 2017-12-22 0:27 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1513839672.25872.13.camel@mhfsdcap03>
On Thu, Dec 21, 2017 at 03:01:12PM +0800, Honghui Zhang wrote:
> On Thu, 2017-12-21 at 14:41 +0800, Yong Wu wrote:
> > On Thu, 2017-12-21 at 10:11 +0800, honghui.zhang at mediatek.com wrote:
> > > From: Honghui Zhang <honghui.zhang@mediatek.com>
> > >
> > > The host bridge of MT7622 has hardware code the class code to an
> > > arbitrary, meaningless value, fix that.
> > >
> > > Signed-off-by: Honghui Zhang <honghui.zhang@mediatek.com>
> > > ---
> > > drivers/pci/host/pcie-mediatek.c | 12 ++++++++++++
> > > 1 file changed, 12 insertions(+)
> > >
> > > diff --git a/drivers/pci/host/pcie-mediatek.c b/drivers/pci/host/pcie-mediatek.c
> > > index 3248771..ae8d367 100644
> > > --- a/drivers/pci/host/pcie-mediatek.c
> > > +++ b/drivers/pci/host/pcie-mediatek.c
> > > @@ -1174,3 +1174,15 @@ static struct platform_driver mtk_pcie_driver = {
> > > },
> > > };
> > > builtin_platform_driver(mtk_pcie_driver);
> > > +
> > > +/* The host bridge of MT7622 advertises the wrong device class. */
> > > +static void mtk_fixup_class(struct pci_dev *dev)
> > > +{
> > > + dev->class = PCI_CLASS_BRIDGE_PCI << 8;
> > > +}
> > > +
> > > +/*
> > > + * The HW default value of vendor id and device id for mt7622 are 0x0e8d,
> > > + * 0x3258, which are arbitrary, meaningless values.
They may be arbitrary but they are certainly not meaningless.
> > What's the right vendor id and device id? is it possible to fix them
> > too?
>
> Vendor ID is managed by PCI-SIG, you may get the assigned vendor ID
> from:
> https://pci-ids.ucw.cz/read/PC?restrict=
It's true that Vendor IDs are managed by the PCI-SIG. The link above
is not managed by the PCI-SIG and is not the official list of assigned
Vendor IDs.
> The vendor ID for Mediatek Corp. should be 14c3.
> Device ID is something like vendor-defined.
> Those values are in the configuration space and are read-only defined by
> spec, it's been stored at the pci_dev, we may change the vendor and
> device values in pci_dev, but I don't think it's necessary to change
> that.
> BTW, Does anyone really cares about the vendor ID and device ID except
> the device's driver?
Yes. This is a fundamental misunderstanding of how Vendor and Device
IDs work. The *driver* really doesn't care about the IDs. The PCI
*core* cares about them because it uses the IDs to select the
appropriate driver to bind to the device.
> > > +DECLARE_PCI_FIXUP_EARLY(0x0e8d, 0x3258, mtk_fixup_class);
This is absolutely not OK. You cannot take over Vendor ID 0x0e8d
unless it has been assigned to you by the PCI-SIG.
To my knowledge, 0x0e8d has not been assigned to any company yet, but
the PCI-SIG could assign it to some new company X tomorrow. Company X
may then build a device with Device ID 0x3258. Now we cannot tell the
difference between the Company X device that is correctly designed and
this Mediatek device that is broken.
This quirk would improperly overwrite dev->class for the Company X
device, and we would not be able to bind the correct driver to either
device.
I am amazed that somebody could build a device that claims to be a PCI
device and get the Vendor ID wrong. That should be literally the
first thing the hardware designer does. If you have hardware in the
field that uses the wrong Vendor ID, it is definitely not
PCI-compliant.
Bjorn
^ permalink raw reply
* [PATCH v6 1/3] clk: at91: pmc: Wait for clocks when resuming
From: Stephen Boyd @ 2017-12-22 0:34 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171211165535.5126-2-romain.izard.pro@gmail.com>
On 12/11, Romain Izard wrote:
> Wait for the syncronization of all clocks when resuming, not only the
> UPLL clock. Do not use regmap_read_poll_timeout, as it will call BUG()
> when interrupts are masked, which is the case in here.
>
> Signed-off-by: Romain Izard <romain.izard.pro@gmail.com>
> Acked-by: Ludovic Desroches <ludovic.desroches@microchip.com>
> Acked-by: Nicolas Ferre <nicolas.ferre@microchip.com>
> ---
Applied to clk-next
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply
* [PATCH v6 2/3] clk: at91: pmc: Save SCSR during suspend
From: Stephen Boyd @ 2017-12-22 0:34 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171211165535.5126-3-romain.izard.pro@gmail.com>
On 12/11, Romain Izard wrote:
> The contents of the System Clock Status Register (SCSR) needs to be
> restored into the System Clock Enable Register (SCER).
>
> As the bootloader will restore some clocks by itself, the issue can be
> missed as only the USB controller, the LCD controller, the Image Sensor
> controller and the programmable clocks will be impacted.
>
> Fix the obvious typo in the suspend/resume code, as the IMR register
> does not need to be saved twice.
>
> Signed-off-by: Romain Izard <romain.izard.pro@gmail.com>
> Acked-by: Nicolas Ferre <nicolas.ferre@microchip.com>
> ---
Applied to clk-next
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply
* [PATCH v6 3/3] clk: at91: pmc: Support backup for programmable clocks
From: Stephen Boyd @ 2017-12-22 0:34 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171211165535.5126-4-romain.izard.pro@gmail.com>
On 12/11, Romain Izard wrote:
> When an AT91 programmable clock is declared in the device tree, register
> it into the Power Management Controller driver. On entering suspend mode,
> the driver saves and restores the Programmable Clock registers to support
> the backup mode for these clocks.
>
> Signed-off-by: Romain Izard <romain.izard.pro@gmail.com>
> Acked-by: Nicolas Ferre <nicolas.ferre@microchip.com>
> ---
Applied to clk-next
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply
* [PATCH v2 2/2] PCI: mediatek: Fixup class type for MT7622
From: Honghui Zhang @ 2017-12-22 0:56 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171222002712.GE30595@bhelgaas-glaptop.roam.corp.google.com>
On Thu, 2017-12-21 at 18:27 -0600, Bjorn Helgaas wrote:
> On Thu, Dec 21, 2017 at 03:01:12PM +0800, Honghui Zhang wrote:
> > On Thu, 2017-12-21 at 14:41 +0800, Yong Wu wrote:
> > > On Thu, 2017-12-21 at 10:11 +0800, honghui.zhang at mediatek.com wrote:
> > > > From: Honghui Zhang <honghui.zhang@mediatek.com>
> > > >
> > > > The host bridge of MT7622 has hardware code the class code to an
> > > > arbitrary, meaningless value, fix that.
> > > >
> > > > Signed-off-by: Honghui Zhang <honghui.zhang@mediatek.com>
> > > > ---
> > > > drivers/pci/host/pcie-mediatek.c | 12 ++++++++++++
> > > > 1 file changed, 12 insertions(+)
> > > >
> > > > diff --git a/drivers/pci/host/pcie-mediatek.c b/drivers/pci/host/pcie-mediatek.c
> > > > index 3248771..ae8d367 100644
> > > > --- a/drivers/pci/host/pcie-mediatek.c
> > > > +++ b/drivers/pci/host/pcie-mediatek.c
> > > > @@ -1174,3 +1174,15 @@ static struct platform_driver mtk_pcie_driver = {
> > > > },
> > > > };
> > > > builtin_platform_driver(mtk_pcie_driver);
> > > > +
> > > > +/* The host bridge of MT7622 advertises the wrong device class. */
> > > > +static void mtk_fixup_class(struct pci_dev *dev)
> > > > +{
> > > > + dev->class = PCI_CLASS_BRIDGE_PCI << 8;
> > > > +}
> > > > +
> > > > +/*
> > > > + * The HW default value of vendor id and device id for mt7622 are 0x0e8d,
> > > > + * 0x3258, which are arbitrary, meaningless values.
>
> They may be arbitrary but they are certainly not meaningless.
>
> > > What's the right vendor id and device id? is it possible to fix them
> > > too?
> >
> > Vendor ID is managed by PCI-SIG, you may get the assigned vendor ID
> > from:
> > https://pci-ids.ucw.cz/read/PC?restrict=
>
> It's true that Vendor IDs are managed by the PCI-SIG. The link above
> is not managed by the PCI-SIG and is not the official list of assigned
> Vendor IDs.
>
> > The vendor ID for Mediatek Corp. should be 14c3.
> > Device ID is something like vendor-defined.
> > Those values are in the configuration space and are read-only defined by
> > spec, it's been stored at the pci_dev, we may change the vendor and
> > device values in pci_dev, but I don't think it's necessary to change
> > that.
> > BTW, Does anyone really cares about the vendor ID and device ID except
> > the device's driver?
>
> Yes. This is a fundamental misunderstanding of how Vendor and Device
> IDs work. The *driver* really doesn't care about the IDs. The PCI
> *core* cares about them because it uses the IDs to select the
> appropriate driver to bind to the device.
>
Thanks, I was wrong about this, I had not seen that the Vendor IDs may
be assigned to another company in the future. I should try another way
to fix this.
> > > > +DECLARE_PCI_FIXUP_EARLY(0x0e8d, 0x3258, mtk_fixup_class);
>
> This is absolutely not OK. You cannot take over Vendor ID 0x0e8d
> unless it has been assigned to you by the PCI-SIG.
>
> To my knowledge, 0x0e8d has not been assigned to any company yet, but
> the PCI-SIG could assign it to some new company X tomorrow. Company X
> may then build a device with Device ID 0x3258. Now we cannot tell the
> difference between the Company X device that is correctly designed and
> this Mediatek device that is broken.
>
> This quirk would improperly overwrite dev->class for the Company X
> device, and we would not be able to bind the correct driver to either
> device.
>
I will try another way to fix this, thanks very much for your explain.
> I am amazed that somebody could build a device that claims to be a PCI
> device and get the Vendor ID wrong. That should be literally the
> first thing the hardware designer does. If you have hardware in the
> field that uses the wrong Vendor ID, it is definitely not
> PCI-compliant.
There's an internal control register that control the Vendor ID and
device ID values, our designer leave the default value un-touched. I
will set these values in that way for the next version of fix.
Thanks.
>
> Bjorn
^ permalink raw reply
* [PATCH V2 01/10] clk: clk-divider: add CLK_DIVIDER_ZERO_GATE clk support
From: Stephen Boyd @ 2017-12-22 1:24 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171220142757.GB30461@b29396-OptiPlex-7040>
On 12/20, Dong Aisheng wrote:
> On Thu, Nov 02, 2017 at 12:50:39AM -0700, Stephen Boyd wrote:
> > On 07/13, Dong Aisheng wrote:
> > > diff --git a/drivers/clk/clk-divider.c b/drivers/clk/clk-divider.c
> > > index 9bb472c..55f8c41 100644
> > > --- a/drivers/clk/clk-divider.c
> > > +++ b/drivers/clk/clk-divider.c
> > > @@ -123,6 +123,9 @@ unsigned long divider_recalc_rate(struct clk_hw *hw, unsigned long parent_rate,
> > > struct clk_divider *divider = to_clk_divider(hw);
> > > unsigned int div;
> > >
> > > + if (flags & CLK_DIVIDER_ZERO_GATE && !val)
> > > + return 0;
> > > +
> > > div = _get_div(table, val, flags, divider->width);
> > > if (!div) {
> > > WARN(!(flags & CLK_DIVIDER_ALLOW_ZERO),
> > > @@ -141,8 +144,13 @@ static unsigned long clk_divider_recalc_rate(struct clk_hw *hw,
> > > struct clk_divider *divider = to_clk_divider(hw);
> > > unsigned int val;
> > >
> > > - val = clk_readl(divider->reg) >> divider->shift;
> > > - val &= div_mask(divider->width);
> > > + if ((divider->flags & CLK_DIVIDER_ZERO_GATE) &&
> > > + !clk_hw_is_enabled(hw)) {
> >
> > This seems racy. Are we holding the register lock here?
> >
>
> Would you please clarify what type of racy you mean?
I mean a race between clk_enable() and clk_set_rate(). A
clk_enable() can happen while a rate change is going on, and then
the clk_hw_is_enabled() check here would be racing, unless this
driver specifically tries to prevent that from happening by
holding a spinlock somewhere.
>
> Currently it only protects register write between set_rate and enable/disable,
> and other register read are not protected.
> e.g. in recalc_rate and is_enabled.
If you're holding some lock that is used to protect the register
writes and also the clk from getting enabled/disabled during a
rate change then it's fine.
>
> And i did see similar users, e.g.
> drivers/clk/sunxi-ng/ccu_mult.c
Sure. Those could also be broken. I'm not sure.
>
> Should we still need protect them here?
>
> > > + val = divider->cached_val;
> > > + } else {
> > > + val = clk_readl(divider->reg) >> divider->shift;
> > > + val &= div_mask(divider->width);
> > > + }
> > >
> > > return divider_recalc_rate(hw, parent_rate, val, divider->table,
> > > divider->flags);
> > > @@ -392,6 +400,12 @@ static int clk_divider_set_rate(struct clk_hw *hw, unsigned long rate,
> > > value = divider_get_val(rate, parent_rate, divider->table,
> > > divider->width, divider->flags);
> > >
> > > + if ((divider->flags & CLK_DIVIDER_ZERO_GATE) &&
> > > + !clk_hw_is_enabled(hw)) {
> >
> > Same racy comment here.
> >
> > > + divider->cached_val = value;
> > > + return 0;
> > > + }
> > > +
> > > if (divider->lock)
> > > spin_lock_irqsave(divider->lock, flags);
> > > else
> > > @@ -414,10 +428,85 @@ static int clk_divider_set_rate(struct clk_hw *hw, unsigned long rate,
> > > return 0;
> > > }
> > >
> > > +static int clk_divider_enable(struct clk_hw *hw)
> > > +{
> > > + struct clk_divider *divider = to_clk_divider(hw);
> > > + unsigned long flags = 0;
> > > + u32 val;
> > > +
> > > + if (!(divider->flags & CLK_DIVIDER_ZERO_GATE))
> > > + return 0;
> >
> > This is not good. We will always jump to these functions on
> > enable/disable for a divider although 99.9% of all dividers that
> > exist won't need to run this code at all.
> >
>
> I absolutely understand this concern.
>
> > Can you please move this logic into your own divider
> > implementation? The flag can be added to the generic layer if
> > necessary but I'd prefer to see this logic kept in the driver
> > that uses it. If we get more than one driver doing the cached
> > divider thing then we can think about moving it to the more
> > generic place like here, but for now we should be able to keep
> > this contained away from the basic types and handled by the
> > quirky driver that needs it.
> >
>
> If only for above issue, how about invent a clk_divider_gate_ops
> to separate the users of normal divider and zero gate divider:
>
> diff --git a/drivers/clk/clk-divider.c b/drivers/clk/clk-divider.c
> index 4ed516c..b51f3f9 100644
> --- a/drivers/clk/clk-divider.c
> +++ b/drivers/clk/clk-divider.c
> @@ -125,6 +125,9 @@ unsigned long divider_recalc_rate(struct clk_hw *hw, unsigned long parent_rate,
>
> div = _get_div(table, val, flags, divider->width);
> if (!div) {
> + if (flags & CLK_DIVIDER_ZERO_GATE)
> + return 0;
> +
> WARN(!(flags & CLK_DIVIDER_ALLOW_ZERO),
> "%s: Zero divisor and CLK_DIVIDER_ALLOW_ZERO not set\n",
> clk_hw_get_name(hw));
> @@ -148,6 +151,23 @@ static unsigned long clk_divider_recalc_rate(struct clk_hw *hw,
> divider->flags);
> }
>
> +static unsigned long clk_divider_gate_recalc_rate(struct clk_hw *hw,
> + unsigned long parent_rate)
> +{
> + struct clk_divider *divider = to_clk_divider(hw);
> + unsigned int val;
> +
> + if (!clk_hw_is_enabled(hw)) {
> + val = divider->cached_val;
> + } else {
> + val = clk_readl(divider->reg) >> divider->shift;
> + val &= div_mask(divider->width);
> + }
> +
> + return divider_recalc_rate(hw, parent_rate, val, divider->table,
> + divider->flags);
> +}
> +
> static bool _is_valid_table_div(const struct clk_div_table *table,
> unsigned int div)
> {
> @@ -416,6 +436,89 @@ static int clk_divider_set_rate(struct clk_hw *hw, unsigned long rate,
> return 0;
> }
>
> +static int clk_divider_gate_set_rate(struct clk_hw *hw, unsigned long rate,
> + unsigned long parent_rate)
> +{
> + struct clk_divider *divider = to_clk_divider(hw);
> + int value;
> +
> + if (!clk_hw_is_enabled(hw)) {
> + value = divider_get_val(rate, parent_rate, divider->table,
> + divider->width, divider->flags);
> + if (value < 0)
> + return value;
> +
> + divider->cached_val = value;
> +
> + return 0;
> + }
> +
> + return clk_divider_set_rate(hw, rate, parent_rate);
> +}
> +
> +static int clk_divider_enable(struct clk_hw *hw)
> +{
> + struct clk_divider *divider = to_clk_divider(hw);
> + unsigned long uninitialized_var(flags);
> + u32 val;
> +
> + if (!divider->cached_val) {
> + pr_err("%s: no valid preset rate\n", clk_hw_get_name(hw));
> + return -EINVAL;
> + }
> +
> + if (divider->lock)
> + spin_lock_irqsave(divider->lock, flags);
> + else
> + __acquire(divider->lock);
> +
> + /* restore div val */
> + val = clk_readl(divider->reg);
> + val |= divider->cached_val << divider->shift;
> + clk_writel(val, divider->reg);
> +
> + if (divider->lock)
> + spin_unlock_irqrestore(divider->lock, flags);
> + else
> + __release(divider->lock);
> +
> + return 0;
> +}
> +
> +static void clk_divider_disable(struct clk_hw *hw)
> +{
> + struct clk_divider *divider = to_clk_divider(hw);
> + unsigned long uninitialized_var(flags);
> + u32 val;
> +
> + if (divider->lock)
> + spin_lock_irqsave(divider->lock, flags);
> + else
> + __acquire(divider->lock);
> +
> + /* store the current div val */
> + val = clk_readl(divider->reg) >> divider->shift;
> + val &= div_mask(divider->width);
> + divider->cached_val = val;
> + clk_writel(0, divider->reg);
> +
> + if (divider->lock)
> + spin_unlock_irqrestore(divider->lock, flags);
> + else
> + __release(divider->lock);
> +}
> +
> +static int clk_divider_is_enabled(struct clk_hw *hw)
> +{
> + struct clk_divider *divider = to_clk_divider(hw);
> + u32 val;
> +
> + val = clk_readl(divider->reg) >> divider->shift;
> + val &= div_mask(divider->width);
> +
> + return val ? 1 : 0;
> +}
> +
> const struct clk_ops clk_divider_ops = {
> .recalc_rate = clk_divider_recalc_rate,
> .round_rate = clk_divider_round_rate,
> @@ -423,6 +526,16 @@ const struct clk_ops clk_divider_ops = {
> };
> EXPORT_SYMBOL_GPL(clk_divider_ops);
>
> +const struct clk_ops clk_divider_gate_ops = {
> + .recalc_rate = clk_divider_gate_recalc_rate,
> + .round_rate = clk_divider_round_rate,
> + .set_rate = clk_divider_gate_set_rate,
> + .enable = clk_divider_enable,
> + .disable = clk_divider_disable,
> + .is_enabled = clk_divider_is_enabled,
> +};
> +EXPORT_SYMBOL_GPL(clk_divider_gate_ops);
> +
> const struct clk_ops clk_divider_ro_ops = {
> .recalc_rate = clk_divider_recalc_rate,
> .round_rate = clk_divider_round_rate,
> @@ -438,6 +551,7 @@ static struct clk_hw *_register_divider(struct device *dev, const char *name,
> struct clk_divider *div;
> struct clk_hw *hw;
> struct clk_init_data init;
> + u32 val;
> int ret;
>
> if (clk_divider_flags & CLK_DIVIDER_HIWORD_MASK) {
> @@ -455,6 +569,8 @@ static struct clk_hw *_register_divider(struct device *dev, const char *name,
> init.name = name;
> if (clk_divider_flags & CLK_DIVIDER_READ_ONLY)
> init.ops = &clk_divider_ro_ops;
> + else if (clk_divider_flags & CLK_DIVIDER_ZERO_GATE)
> + init.ops = &clk_divider_gate_ops;
> else
> init.ops = &clk_divider_ops;
> init.flags = flags | CLK_IS_BASIC;
> @@ -470,6 +586,12 @@ static struct clk_hw *_register_divider(struct device *dev, const char *name,
> div->hw.init = &init;
> div->table = table;
>
> + if (div->flags & CLK_DIVIDER_ZERO_GATE) {
> + val = clk_readl(reg) >> shift;
> + val &= div_mask(width);
> + div->cached_val = val;
> + }
> +
> /* register the clock */
> hw = &div->hw;
> ret = clk_hw_register(dev, hw);
> diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
> index 7c925e6..5f33b73 100644
> --- a/include/linux/clk-provider.h
> +++ b/include/linux/clk-provider.h
> @@ -358,6 +358,7 @@ struct clk_div_table {
> * @shift: shift to the divider bit field
> * @width: width of the divider bit field
> * @table: array of value/divider pairs, last entry should have div = 0
> + * @cached_val: cached div hw value used for CLK_DIVIDER_ZERO_GATE
> * @lock: register lock
> *
> * Clock with an adjustable divider affecting its output frequency. Implements
> @@ -386,6 +387,12 @@ struct clk_div_table {
> * CLK_DIVIDER_MAX_AT_ZERO - For dividers which are like CLK_DIVIDER_ONE_BASED
> * except when the value read from the register is zero, the divisor is
> * 2^width of the field.
> + * CLK_DIVIDER_ZERO_GATE - For dividers which are like CLK_DIVIDER_ONE_BASED
> + * when the value read from the register is zero, it means the divisor
> + * is gated. For this case, the cached_val will be used to store the
> + * intermediate div for the normal rate operation, like set_rate/get_rate/
> + * recalc_rate. When the divider is ungated, the driver will actually
> + * program the hardware to have the requested divider value.
> */
> struct clk_divider {
> struct clk_hw hw;
> @@ -394,6 +401,7 @@ struct clk_divider {
> u8 width;
> u8 flags;
> const struct clk_div_table *table;
> + u32 cached_val;
> spinlock_t *lock;
> };
>
> @@ -406,6 +414,7 @@ struct clk_divider {
> #define CLK_DIVIDER_ROUND_CLOSEST BIT(4)
> #define CLK_DIVIDER_READ_ONLY BIT(5)
> #define CLK_DIVIDER_MAX_AT_ZERO BIT(6)
> +#define CLK_DIVIDER_ZERO_GATE BIT(7)
>
> extern const struct clk_ops clk_divider_ops;
> extern const struct clk_ops clk_divider_ro_ops;
>
> Anyway, if you still think it's not proper, i can put it in platform
> driver as you wish, just in the cost of a few duplicated codes.
Ok. Keeping it in the basic types but split into different ops
path looks good.
>
> > > +
> > > + if (!divider->cached_val) {
> > > + pr_err("%s: no valid preset rate\n", clk_hw_get_name(hw));
> > > + return -EINVAL;
> > > + }
> > > +
> > > + if (divider->lock)
> > > + spin_lock_irqsave(divider->lock, flags);
> > > + else
> > > + __acquire(divider->lock);
> > > +
> > > + /* restore div val */
> > > + val = clk_readl(divider->reg);
> > > + val |= divider->cached_val << divider->shift;
> > > + clk_writel(val, divider->reg);
> > > +
> > > + if (divider->lock)
> > > + spin_unlock_irqrestore(divider->lock, flags);
> > > + else
> > > + __release(divider->lock);
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static void clk_divider_disable(struct clk_hw *hw)
> > > +{
> > > + struct clk_divider *divider = to_clk_divider(hw);
> > > + unsigned long flags = 0;
> > > + u32 val;
> > > +
> > > + if (!(divider->flags & CLK_DIVIDER_ZERO_GATE))
> > > + return;
> > > +
> > > + if (divider->lock)
> > > + spin_lock_irqsave(divider->lock, flags);
> > > + else
> > > + __acquire(divider->lock);
> > > +
> > > + /* store the current div val */
> > > + val = clk_readl(divider->reg) >> divider->shift;
> > > + val &= div_mask(divider->width);
> > > + divider->cached_val = val;
> > > + clk_writel(0, divider->reg);
> > > +
> > > + if (divider->lock)
> > > + spin_unlock_irqrestore(divider->lock, flags);
> > > + else
> > > + __release(divider->lock);
> > > +}
> > > +
> > > +static int clk_divider_is_enabled(struct clk_hw *hw)
> > > +{
> > > + struct clk_divider *divider = to_clk_divider(hw);
> > > + u32 val;
> > > +
> > > + if (!(divider->flags & CLK_DIVIDER_ZERO_GATE))
> > > + return __clk_get_enable_count(hw->clk);
> >
> > The plan was to delete this API once OMAP stopped using it.
> > clk_hw_is_enabled() doesn't work?
>
> No, it did not work before because clk_hw_is_enabled will result
> in the dead loop by calling .is_enabled() callback again.
>
> That's why __clk_get_enable_count is used instead.
>
> However, with above new patch method, this issue was gone.
Great!
>
> >
> > > +
> > > + val = clk_readl(divider->reg) >> divider->shift;
> > > + val &= div_mask(divider->width);
> > > +
> > > + return val ? 1 : 0;
> > > +}
> > > +
> > > const struct clk_ops clk_divider_ops = {
> > > .recalc_rate = clk_divider_recalc_rate,
> > > .round_rate = clk_divider_round_rate,
> > > .set_rate = clk_divider_set_rate,
> > > + .enable = clk_divider_enable,
> > > + .disable = clk_divider_disable,
> > > + .is_enabled = clk_divider_is_enabled,
> > > };
> > > EXPORT_SYMBOL_GPL(clk_divider_ops);
> > >
> > > @@ -436,6 +525,7 @@ static struct clk_hw *_register_divider(struct device *dev, const char *name,
> > > struct clk_divider *div;
> > > struct clk_hw *hw;
> > > struct clk_init_data init;
> > > + u32 val;
> > > int ret;
> > >
> > > if (clk_divider_flags & CLK_DIVIDER_HIWORD_MASK) {
> > > @@ -468,6 +558,12 @@ static struct clk_hw *_register_divider(struct device *dev, const char *name,
> > > div->hw.init = &init;
> > > div->table = table;
> > >
> > > + if (div->flags & CLK_DIVIDER_ZERO_GATE) {
> > > + val = clk_readl(reg) >> shift;
> > > + val &= div_mask(width);
> > > + div->cached_val = val;
> > > + }
> >
> > What if it isn't on? Setting cached_val to 0 is ok?
> >
>
> If it isn't on, then the cache_val should be 0.
> And recalc_rate will catch this case and return 0 as there's
> no proper pre-set rate.
>
Ok.
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply
* [PATCH v6 0/5] clk: Add Aspeed clock driver
From: Joel Stanley @ 2017-12-22 1:42 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171221234043.GF7997@codeaurora.org>
On Fri, Dec 22, 2017 at 10:10 AM, Stephen Boyd <sboyd@codeaurora.org> wrote:
> On 12/06, Joel Stanley wrote:
>> Hello clk maintainers,
>>
>> Has anyone had a chance to review these patches?
>>
>
> Besides the sleeping call that should be a delay it looks OK.
> Send v7?
Thanks for taking a look. I'll fix the sleep call and send it out now.
Cheers,
Joel
^ permalink raw reply
* [PATCH 3/3] [v6] pinctrl: qcom: qdf2xxx: add support for new ACPI HID QCOM8002
From: Stephen Boyd @ 2017-12-22 1:46 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <38463c81-eeab-85dc-d197-6081ce1d1130@codeaurora.org>
On 12/20, Timur Tabi wrote:
> On 12/20/17 6:39 PM, Stephen Boyd wrote:
> >I don't see how it hurts to treat it generically. Presumably
> >that's the way it will be done on ACPI platforms going forward?
> >No need to tie it to some ACPI HID.
>
> But it is tied to a HID. The "num-gpios" and "gpios" properties
> belong to a specific HID. Someone could create a new HID with
> different properties, and then what? That's why I want all the ACPI
> stuff in the client driver.
>
> At this point I don't really care any more about what the patches
> look like, but I really do think that putting the ACPI code in
> pinctrl-msm is a bad idea.
>
> We're debating adding support for multiple TLMMs, and we may create
> a new HID for that, so that we can define all pins on all TLMMs in
> one device. We would need to create a new HID and new DSDs to go
> with it.
Ok. That's testable with acpi_match_device_ids() though. I can
add that into pinctrl-msm.c so we don't have to pass info about
available gpios from ACPI specific driver into the pinctrl-msm
core driver. That's why I'm trying to avoid doing it in the ACPI
specific driver. Do it close to where the gpiochip is created
instead.
Maybe future HIDs could follow the DT design and then we can look
for the same device property name in both firmwares. Parsing
ranges is simpler.
>
> >I'm trying to resolve everything at once: gpios, pinctrl pins,
> >and irqs exposed by the TLMM hardware. The value is that we solve
> >it all, once, now.
>
> Keep in mind that I am now in vacation, and so I won't be able to
> submit any more patches for a while.
>
> >The DT binding can also be resolved at the
> >same time, so when we need to express this in DT it's already
> >done.
>
> Ok.
>
> >Otherwise, something can request irqs from the irqdomain
> >even if the irq can't be enabled, or it can try to mux the pin to
> >some other function, even if the function selection can't be
> >configured.
>
> Is it possible to request an IRQ for a pin if the pin itself can't
> be requested?
I don't see any place blocking GPIOs turning into IRQs once we
setup the irqdomain with interrupts. Maybe I missed something,
but I think you can request an IRQ once the domain has the hwirqs
associated with it. I will test it out.
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply
* [PATCH V2 02/10] clk: reparent orphans after critical clocks enabled
From: Stephen Boyd @ 2017-12-22 1:55 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171221183233.GK7997@codeaurora.org>
On 12/21, Stephen Boyd wrote:
>
> Ok. Great. I'm going to apply this patch now into clk-next to
> look for regressions on other platforms. If this was the only
> questionable thing about this series then I think I can apply the
> rest of it without you needing to resend. I'll check today.
>
I pushed it into clk-imx7ulp and merged that into clk-next. You
can use that patch in your series.
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply
* [PATCH 01/10] soc: qcom: Separate kryo l2 accessors from PMU driver
From: Stephen Boyd @ 2017-12-22 2:06 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171212140302.au4wart4uhwm7lfq@lakrids.cambridge.arm.com>
On 12/12, Mark Rutland wrote:
> On Tue, Dec 12, 2017 at 02:31:28PM +0200, Ilia Lin wrote:
> > + * accesses, and system registers with respect to device memory
> > + */
> > +void set_l2_indirect_reg(u64 reg, u64 val)
> > +{
> > + unsigned long flags;
> > + mb();
>
> We didn't need this for the PMU driver, so it's unfortuante that it now
> has to pay the cost.
>
> Can we please factor this mb() into the callers that need it?
+1
>
> > + raw_spin_lock_irqsave(&l2_access_lock, flags);
> > + write_sysreg_s(reg, L2CPUSRSELR_EL1);
> > + isb();
> > + write_sysreg_s(val, L2CPUSRDR_EL1);
> > + isb();
> > + raw_spin_unlock_irqrestore(&l2_access_lock, flags);
> > +}
> > +EXPORT_SYMBOL(set_l2_indirect_reg);
>
> [...]
>
> > +#ifdef CONFIG_ARCH_QCOM
> > +void set_l2_indirect_reg(u64 reg_addr, u64 val);
> > +u64 get_l2_indirect_reg(u64 reg_addr);
> > +#else
> > +static inline void set_l2_indirect_reg(u32 reg_addr, u32 val) {}
> > +static inline u32 get_l2_indirect_reg(u32 reg_addr)
> > +{
> > + return 0;
> > +}
> > +#endif
> > +#endif
>
> Are there any drivers that will bne built for !CONFIG_ARCH_QCOM that
> reference this?
>
> It might be better to not have the stub versions, so that we get a
> build-error if they are erroneously used.
>
Does that approach make COMPILE_TEST break?
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply
* [PATCH v6 4/5] clk: aspeed: Register gated clocks
From: Benjamin Herrenschmidt @ 2017-12-22 2:36 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171221233927.GE7997@codeaurora.org>
On Thu, 2017-12-21 at 15:39 -0800, Stephen Boyd wrote:
> On 11/28, Joel Stanley wrote:
> > diff --git a/drivers/clk/clk-aspeed.c b/drivers/clk/clk-aspeed.c
> > index 839243691b26..b5dc3e298693 100644
> > --- a/drivers/clk/clk-aspeed.c
> > +++ b/drivers/clk/clk-aspeed.c
> > @@ -202,6 +202,107 @@ static const struct aspeed_clk_soc_data ast2400_data = {
> > .calc_pll = aspeed_ast2400_calc_pll,
> > };
> >
> > +static int aspeed_clk_enable(struct clk_hw *hw)
> > +{
> > + struct aspeed_clk_gate *gate = to_aspeed_clk_gate(hw);
> > + unsigned long flags;
> > + u32 clk = BIT(gate->clock_idx);
> > + u32 rst = BIT(gate->reset_idx);
> > +
> > + spin_lock_irqsave(gate->lock, flags);
> > +
> > + if (gate->reset_idx >= 0) {
> > + /* Put IP in reset */
> > + regmap_update_bits(gate->map, ASPEED_RESET_CTRL, rst, rst);
> > +
> > + /* Delay 100us */
> > + udelay(100);
> > + }
> > +
> > + /* Enable clock */
> > + regmap_update_bits(gate->map, ASPEED_CLK_STOP_CTRL, clk, 0);
> > +
> > + if (gate->reset_idx >= 0) {
> > + /* Delay 10ms */
> > + /* TODO: can we sleep here? */
> > + msleep(10);
>
> No you can't sleep here. It needs to delay because this is inside
> spinlock_irqsave.
Additionally you really don't want to delay for 10ms with interrupts
off :-(
Sadly, it looks like the clk framework already calls you with spinlock
irqsafe, which is a rather major suckage.
Stephen, why is that so ? That pretty much makes it impossible to
do sleeping things, which prevents things like i2c based clock
controllers etc...
I think the clk framework needs to be overhauled to use sleeping
mutexes instead. Doing clock enable/disable at interrupt time is
a bad idea anyway.
In the meantime, Joel, you have little choice but do an mdelay
though that really sucks.
> > + /* Take IP out of reset */
> > + regmap_update_bits(gate->map, ASPEED_RESET_CTRL, rst, 0);
> > + }
> > +
> > + spin_unlock_irqrestore(gate->lock, flags);
> > +
> > + return 0;
> > +}
^ permalink raw reply
* [PATCH v6 4/5] clk: aspeed: Register gated clocks
From: Benjamin Herrenschmidt @ 2017-12-22 2:43 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1513910191.2743.77.camel@kernel.crashing.org>
On Fri, 2017-12-22 at 13:36 +1100, Benjamin Herrenschmidt wrote:
>
> > No you can't sleep here. It needs to delay because this is inside
> > spinlock_irqsave.
>
> Additionally you really don't want to delay for 10ms with interrupts
> off :-(
>
> Sadly, it looks like the clk framework already calls you with spinlock
> irqsafe, which is a rather major suckage.
>
> Stephen, why is that so ? That pretty much makes it impossible to
> do sleeping things, which prevents things like i2c based clock
> controllers etc...
I noticed we do have a few i2c based clock drivers... how are they ever
supposed to work ? i2c bus controllers are allowed to sleep and the i2c
core takes mutexes...
> I think the clk framework needs to be overhauled to use sleeping
> mutexes instead. Doing clock enable/disable at interrupt time is
> a bad idea anyway.
>
>
> In the meantime, Joel, you have little choice but do an mdelay
> though that really sucks.
>
> > > + /* Take IP out of reset */
> > > + regmap_update_bits(gate->map, ASPEED_RESET_CTRL, rst, 0);
> > > + }
> > > +
> > > + spin_unlock_irqrestore(gate->lock, flags);
> > > +
> > > + return 0;
> > > +}
^ permalink raw reply
* [PATCH v7 0/5] clk: Add Aspeed clock driver
From: Joel Stanley @ 2017-12-22 2:45 UTC (permalink / raw)
To: linux-arm-kernel
This driver supports the ast2500, ast2400 (and derivative) BMC SoCs from
Aspeed.
Clk maintainers, this patch set requires this signed tag to be merged first:
git://git.kernel.org/pub/scm/linux/kernel/git/joel/aspeed.git tags/aspeed-4.16-clk-binding
It contains a single commit on top of 4.15-rc1 that will also be merged
via the ARM SoC tree. It has been reviewed by Arnd and Rob.
This is v7. See patches for detailed changelogs.
v7: Address reivew from Stephen and device tree changes
v6: Added reviewed-bys
v5: Address review from Andrew
v4: Address review from Andrew and Stephen.
v3: Address review from Andrew and has seen more testing on hardware
v2: split the driver out into a series of patches to make them easier to
review.
All of the important clocks are supported, with most non-essential ones
also implemented where information is available. I am working with
Aspeed to clear up some of the missing information, including the
missing parent-sibling relationships.
We need to know the rate of the apb clock in order to correctly program
the clocksource driver, so the apb and it's parents are created in the
CLK_OF_DECLARE_DRIVER callback.
The rest of the clocks are created at normal driver probe time. I
followed the Gemini driver's lead with using the regmap where I could,
but also having a pointer to the base address for use with the common
clock callbacks.
The driver borrows from the clk_gate common clock infrastructure, but modifies
it in order to support the clock gate and reset pair that most of the clocks
have. This pair must be reset-ungated-released, with appropriate delays,
according to the datasheet.
The first patch introduces the core clock registration parts, and describes
the clocks. The second creates the core clocks, giving the system enough to
boot (but without uart). Next come the non-core clocks, and finally the reset
controller that is used for the few cocks that don't have a gate to go with their
reset pair.
Please review!
Cheers,
Joel
Joel Stanley (5):
clk: Add clock driver for ASPEED BMC SoCs
clk: aspeed: Register core clocks
clk: aspeed: Add platform driver and register PLLs
clk: aspeed: Register gated clocks
clk: aspeed: Add reset controller
drivers/clk/Kconfig | 12 +
drivers/clk/Makefile | 1 +
drivers/clk/clk-aspeed.c | 658 +++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 671 insertions(+)
create mode 100644 drivers/clk/clk-aspeed.c
--
2.15.1
^ permalink raw reply
* [PATCH v7 1/5] clk: Add clock driver for ASPEED BMC SoCs
From: Joel Stanley @ 2017-12-22 2:45 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171222024522.10362-1-joel@jms.id.au>
This adds the stub of a driver for the ASPEED SoCs. The clocks are
defined and the static registration is set up.
Reviewed-by: Andrew Jeffery <andrew@aj.id.au>
Signed-off-by: Joel Stanley <joel@jms.id.au>
---
v7:
- Rebase on dt-bindings patch
- Add ASPEED_NUM_CLKS as it no longer lives in the header
v6:
- Add SPDX copyright notices
v5:
- Add Andrew's reviewed-by
- Make aspeed_gates not initconst to avoid section mismatch warning
v3:
- use named initlisers for aspeed_gates table
- fix clocks typo
- Move ASPEED_NUM_CLKS to the bottom of the list
- Put gates at the start of the list, so we can use them to initalise
the aspeed_gates table
- Add ASPEED_CLK_SELECTION_2
- Set parent of network MAC gates
---
drivers/clk/Kconfig | 12 ++++
drivers/clk/Makefile | 1 +
drivers/clk/clk-aspeed.c | 141 +++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 154 insertions(+)
create mode 100644 drivers/clk/clk-aspeed.c
diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
index 1c4e1aa6767e..9abe063ef8d2 100644
--- a/drivers/clk/Kconfig
+++ b/drivers/clk/Kconfig
@@ -142,6 +142,18 @@ config COMMON_CLK_GEMINI
This driver supports the SoC clocks on the Cortina Systems Gemini
platform, also known as SL3516 or CS3516.
+config COMMON_CLK_ASPEED
+ bool "Clock driver for Aspeed BMC SoCs"
+ depends on ARCH_ASPEED || COMPILE_TEST
+ default ARCH_ASPEED
+ select MFD_SYSCON
+ select RESET_CONTROLLER
+ ---help---
+ This driver supports the SoC clocks on the Aspeed BMC platforms.
+
+ The G4 and G5 series, including the ast2400 and ast2500, are supported
+ by this driver.
+
config COMMON_CLK_S2MPS11
tristate "Clock driver for S2MPS1X/S5M8767 MFD"
depends on MFD_SEC_CORE || COMPILE_TEST
diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index f7f761b02bed..d260da4809f8 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -27,6 +27,7 @@ obj-$(CONFIG_ARCH_CLPS711X) += clk-clps711x.o
obj-$(CONFIG_COMMON_CLK_CS2000_CP) += clk-cs2000-cp.o
obj-$(CONFIG_ARCH_EFM32) += clk-efm32gg.o
obj-$(CONFIG_COMMON_CLK_GEMINI) += clk-gemini.o
+obj-$(CONFIG_COMMON_CLK_ASPEED) += clk-aspeed.o
obj-$(CONFIG_ARCH_HIGHBANK) += clk-highbank.o
obj-$(CONFIG_CLK_HSDK) += clk-hsdk-pll.o
obj-$(CONFIG_COMMON_CLK_MAX77686) += clk-max77686.o
diff --git a/drivers/clk/clk-aspeed.c b/drivers/clk/clk-aspeed.c
new file mode 100644
index 000000000000..7a86ee08ea4f
--- /dev/null
+++ b/drivers/clk/clk-aspeed.c
@@ -0,0 +1,141 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+#define pr_fmt(fmt) "clk-aspeed: " fmt
+
+#include <linux/clk-provider.h>
+#include <linux/mfd/syscon.h>
+#include <linux/of_address.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+#include <linux/spinlock.h>
+
+#include <dt-bindings/clock/aspeed-clock.h>
+
+#define ASPEED_NUM_CLKS 35
+
+#define ASPEED_STRAP 0x70
+
+/* Keeps track of all clocks */
+static struct clk_hw_onecell_data *aspeed_clk_data;
+
+static void __iomem *scu_base;
+
+/**
+ * struct aspeed_gate_data - Aspeed gated clocks
+ * @clock_idx: bit used to gate this clock in the clock register
+ * @reset_idx: bit used to reset this IP in the reset register. -1 if no
+ * reset is required when enabling the clock
+ * @name: the clock name
+ * @parent_name: the name of the parent clock
+ * @flags: standard clock framework flags
+ */
+struct aspeed_gate_data {
+ u8 clock_idx;
+ s8 reset_idx;
+ const char *name;
+ const char *parent_name;
+ unsigned long flags;
+};
+
+/**
+ * struct aspeed_clk_gate - Aspeed specific clk_gate structure
+ * @hw: handle between common and hardware-specific interfaces
+ * @reg: register controlling gate
+ * @clock_idx: bit used to gate this clock in the clock register
+ * @reset_idx: bit used to reset this IP in the reset register. -1 if no
+ * reset is required when enabling the clock
+ * @flags: hardware-specific flags
+ * @lock: register lock
+ *
+ * Some of the clocks in the Aspeed SoC must be put in reset before enabling.
+ * This modified version of clk_gate allows an optional reset bit to be
+ * specified.
+ */
+struct aspeed_clk_gate {
+ struct clk_hw hw;
+ struct regmap *map;
+ u8 clock_idx;
+ s8 reset_idx;
+ u8 flags;
+ spinlock_t *lock;
+};
+
+#define to_aspeed_clk_gate(_hw) container_of(_hw, struct aspeed_clk_gate, hw)
+
+/* TODO: ask Aspeed about the actual parent data */
+static const struct aspeed_gate_data aspeed_gates[] = {
+ /* clk rst name parent flags */
+ [ASPEED_CLK_GATE_ECLK] = { 0, -1, "eclk-gate", "eclk", 0 }, /* Video Engine */
+ [ASPEED_CLK_GATE_GCLK] = { 1, 7, "gclk-gate", NULL, 0 }, /* 2D engine */
+ [ASPEED_CLK_GATE_MCLK] = { 2, -1, "mclk-gate", "mpll", CLK_IS_CRITICAL }, /* SDRAM */
+ [ASPEED_CLK_GATE_VCLK] = { 3, 6, "vclk-gate", NULL, 0 }, /* Video Capture */
+ [ASPEED_CLK_GATE_BCLK] = { 4, 10, "bclk-gate", "bclk", 0 }, /* PCIe/PCI */
+ [ASPEED_CLK_GATE_DCLK] = { 5, -1, "dclk-gate", NULL, 0 }, /* DAC */
+ [ASPEED_CLK_GATE_REFCLK] = { 6, -1, "refclk-gate", "clkin", CLK_IS_CRITICAL },
+ [ASPEED_CLK_GATE_USBPORT2CLK] = { 7, 3, "usb-port2-gate", NULL, 0 }, /* USB2.0 Host port 2 */
+ [ASPEED_CLK_GATE_LCLK] = { 8, 5, "lclk-gate", NULL, 0 }, /* LPC */
+ [ASPEED_CLK_GATE_USBUHCICLK] = { 9, 15, "usb-uhci-gate", NULL, 0 }, /* USB1.1 (requires port 2 enabled) */
+ [ASPEED_CLK_GATE_D1CLK] = { 10, 13, "d1clk-gate", NULL, 0 }, /* GFX CRT */
+ [ASPEED_CLK_GATE_YCLK] = { 13, 4, "yclk-gate", NULL, 0 }, /* HAC */
+ [ASPEED_CLK_GATE_USBPORT1CLK] = { 14, 14, "usb-port1-gate", NULL, 0 }, /* USB2 hub/USB2 host port 1/USB1.1 dev */
+ [ASPEED_CLK_GATE_UART1CLK] = { 15, -1, "uart1clk-gate", "uart", 0 }, /* UART1 */
+ [ASPEED_CLK_GATE_UART2CLK] = { 16, -1, "uart2clk-gate", "uart", 0 }, /* UART2 */
+ [ASPEED_CLK_GATE_UART5CLK] = { 17, -1, "uart5clk-gate", "uart", 0 }, /* UART5 */
+ [ASPEED_CLK_GATE_ESPICLK] = { 19, -1, "espiclk-gate", NULL, 0 }, /* eSPI */
+ [ASPEED_CLK_GATE_MAC1CLK] = { 20, 11, "mac1clk-gate", "mac", 0 }, /* MAC1 */
+ [ASPEED_CLK_GATE_MAC2CLK] = { 21, 12, "mac2clk-gate", "mac", 0 }, /* MAC2 */
+ [ASPEED_CLK_GATE_RSACLK] = { 24, -1, "rsaclk-gate", NULL, 0 }, /* RSA */
+ [ASPEED_CLK_GATE_UART3CLK] = { 25, -1, "uart3clk-gate", "uart", 0 }, /* UART3 */
+ [ASPEED_CLK_GATE_UART4CLK] = { 26, -1, "uart4clk-gate", "uart", 0 }, /* UART4 */
+ [ASPEED_CLK_GATE_SDCLKCLK] = { 27, 16, "sdclk-gate", NULL, 0 }, /* SDIO/SD */
+ [ASPEED_CLK_GATE_LHCCLK] = { 28, -1, "lhclk-gate", "lhclk", 0 }, /* LPC master/LPC+ */
+};
+
+static void __init aspeed_cc_init(struct device_node *np)
+{
+ struct regmap *map;
+ u32 val;
+ int ret;
+ int i;
+
+ scu_base = of_iomap(np, 0);
+ if (IS_ERR(scu_base))
+ return;
+
+ aspeed_clk_data = kzalloc(sizeof(*aspeed_clk_data) +
+ sizeof(*aspeed_clk_data->hws) * ASPEED_NUM_CLKS,
+ GFP_KERNEL);
+ if (!aspeed_clk_data)
+ return;
+
+ /*
+ * This way all clocks fetched before the platform device probes,
+ * except those we assign here for early use, will be deferred.
+ */
+ for (i = 0; i < ASPEED_NUM_CLKS; i++)
+ aspeed_clk_data->hws[i] = ERR_PTR(-EPROBE_DEFER);
+
+ map = syscon_node_to_regmap(np);
+ if (IS_ERR(map)) {
+ pr_err("no syscon regmap\n");
+ return;
+ }
+ /*
+ * We check that the regmap works on this very first access,
+ * but as this is an MMIO-backed regmap, subsequent regmap
+ * access is not going to fail and we skip error checks from
+ * this point.
+ */
+ ret = regmap_read(map, ASPEED_STRAP, &val);
+ if (ret) {
+ pr_err("failed to read strapping register\n");
+ return;
+ }
+
+ aspeed_clk_data->num = ASPEED_NUM_CLKS;
+ ret = of_clk_add_hw_provider(np, of_clk_hw_onecell_get, aspeed_clk_data);
+ if (ret)
+ pr_err("failed to add DT provider: %d\n", ret);
+};
+CLK_OF_DECLARE_DRIVER(aspeed_cc_g5, "aspeed,ast2500-scu", aspeed_cc_init);
+CLK_OF_DECLARE_DRIVER(aspeed_cc_g4, "aspeed,ast2400-scu", aspeed_cc_init);
--
2.15.1
^ permalink raw reply related
* [PATCH v7 2/5] clk: aspeed: Register core clocks
From: Joel Stanley @ 2017-12-22 2:45 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171222024522.10362-1-joel@jms.id.au>
This registers the core clocks; those which are required to calculate
the rate of the timer peripheral so the system can load a clocksource
driver.
Reviewed-by: Andrew Jeffery <andrew@aj.id.au>
Signed-off-by: Joel Stanley <joel@jms.id.au>
---
v5:
- Add Andrew's Reviewed-by
v4:
- Add defines to document the BIT() macros
v3:
- Fix ast2400 ahb calculation
- Remove incorrect 'this is wrong' comment
- Separate out clkin calc to be per platform
- Support 48MHz clkin on ast2400
---
drivers/clk/clk-aspeed.c | 177 +++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 177 insertions(+)
diff --git a/drivers/clk/clk-aspeed.c b/drivers/clk/clk-aspeed.c
index 7a86ee08ea4f..5adedda82d26 100644
--- a/drivers/clk/clk-aspeed.c
+++ b/drivers/clk/clk-aspeed.c
@@ -13,7 +13,23 @@
#define ASPEED_NUM_CLKS 35
+#define ASPEED_RESET_CTRL 0x04
+#define ASPEED_CLK_SELECTION 0x08
+#define ASPEED_CLK_STOP_CTRL 0x0c
+#define ASPEED_MPLL_PARAM 0x20
+#define ASPEED_HPLL_PARAM 0x24
+#define AST2500_HPLL_BYPASS_EN BIT(20)
+#define AST2400_HPLL_STRAPPED BIT(18)
+#define AST2400_HPLL_BYPASS_EN BIT(17)
+#define ASPEED_MISC_CTRL 0x2c
+#define UART_DIV13_EN BIT(12)
#define ASPEED_STRAP 0x70
+#define CLKIN_25MHZ_EN BIT(23)
+#define AST2400_CLK_SOURCE_SEL BIT(18)
+#define ASPEED_CLK_SELECTION_2 0xd8
+
+/* Globally visible clocks */
+static DEFINE_SPINLOCK(aspeed_clk_lock);
/* Keeps track of all clocks */
static struct clk_hw_onecell_data *aspeed_clk_data;
@@ -91,6 +107,160 @@ static const struct aspeed_gate_data aspeed_gates[] = {
[ASPEED_CLK_GATE_LHCCLK] = { 28, -1, "lhclk-gate", "lhclk", 0 }, /* LPC master/LPC+ */
};
+static const struct clk_div_table ast2400_div_table[] = {
+ { 0x0, 2 },
+ { 0x1, 4 },
+ { 0x2, 6 },
+ { 0x3, 8 },
+ { 0x4, 10 },
+ { 0x5, 12 },
+ { 0x6, 14 },
+ { 0x7, 16 },
+ { 0 }
+};
+
+static const struct clk_div_table ast2500_div_table[] = {
+ { 0x0, 4 },
+ { 0x1, 8 },
+ { 0x2, 12 },
+ { 0x3, 16 },
+ { 0x4, 20 },
+ { 0x5, 24 },
+ { 0x6, 28 },
+ { 0x7, 32 },
+ { 0 }
+};
+
+static struct clk_hw *aspeed_ast2400_calc_pll(const char *name, u32 val)
+{
+ unsigned int mult, div;
+
+ if (val & AST2400_HPLL_BYPASS_EN) {
+ /* Pass through mode */
+ mult = div = 1;
+ } else {
+ /* F = 24Mhz * (2-OD) * [(N + 2) / (D + 1)] */
+ u32 n = (val >> 5) & 0x3f;
+ u32 od = (val >> 4) & 0x1;
+ u32 d = val & 0xf;
+
+ mult = (2 - od) * (n + 2);
+ div = d + 1;
+ }
+ return clk_hw_register_fixed_factor(NULL, name, "clkin", 0,
+ mult, div);
+};
+
+static struct clk_hw *aspeed_ast2500_calc_pll(const char *name, u32 val)
+{
+ unsigned int mult, div;
+
+ if (val & AST2500_HPLL_BYPASS_EN) {
+ /* Pass through mode */
+ mult = div = 1;
+ } else {
+ /* F = clkin * [(M+1) / (N+1)] / (P + 1) */
+ u32 p = (val >> 13) & 0x3f;
+ u32 m = (val >> 5) & 0xff;
+ u32 n = val & 0x1f;
+
+ mult = (m + 1) / (n + 1);
+ div = p + 1;
+ }
+
+ return clk_hw_register_fixed_factor(NULL, name, "clkin", 0,
+ mult, div);
+}
+
+static void __init aspeed_ast2400_cc(struct regmap *map)
+{
+ struct clk_hw *hw;
+ u32 val, freq, div;
+
+ /*
+ * CLKIN is the crystal oscillator, 24, 48 or 25MHz selected by
+ * strapping
+ */
+ regmap_read(map, ASPEED_STRAP, &val);
+ if (val & CLKIN_25MHZ_EN)
+ freq = 25000000;
+ else if (val & AST2400_CLK_SOURCE_SEL)
+ freq = 48000000;
+ else
+ freq = 24000000;
+ hw = clk_hw_register_fixed_rate(NULL, "clkin", NULL, 0, freq);
+ pr_debug("clkin @%u MHz\n", freq / 1000000);
+
+ /*
+ * High-speed PLL clock derived from the crystal. This the CPU clock,
+ * and we assume that it is enabled
+ */
+ regmap_read(map, ASPEED_HPLL_PARAM, &val);
+ WARN(val & AST2400_HPLL_STRAPPED, "hpll is strapped not configured");
+ aspeed_clk_data->hws[ASPEED_CLK_HPLL] = aspeed_ast2400_calc_pll("hpll", val);
+
+ /*
+ * Strap bits 11:10 define the CPU/AHB clock frequency ratio (aka HCLK)
+ * 00: Select CPU:AHB = 1:1
+ * 01: Select CPU:AHB = 2:1
+ * 10: Select CPU:AHB = 4:1
+ * 11: Select CPU:AHB = 3:1
+ */
+ regmap_read(map, ASPEED_STRAP, &val);
+ val = (val >> 10) & 0x3;
+ div = val + 1;
+ if (div == 3)
+ div = 4;
+ else if (div == 4)
+ div = 3;
+ hw = clk_hw_register_fixed_factor(NULL, "ahb", "hpll", 0, 1, div);
+ aspeed_clk_data->hws[ASPEED_CLK_AHB] = hw;
+
+ /* APB clock clock selection register SCU08 (aka PCLK) */
+ hw = clk_hw_register_divider_table(NULL, "apb", "hpll", 0,
+ scu_base + ASPEED_CLK_SELECTION, 23, 3, 0,
+ ast2400_div_table,
+ &aspeed_clk_lock);
+ aspeed_clk_data->hws[ASPEED_CLK_APB] = hw;
+}
+
+static void __init aspeed_ast2500_cc(struct regmap *map)
+{
+ struct clk_hw *hw;
+ u32 val, freq, div;
+
+ /* CLKIN is the crystal oscillator, 24 or 25MHz selected by strapping */
+ regmap_read(map, ASPEED_STRAP, &val);
+ if (val & CLKIN_25MHZ_EN)
+ freq = 25000000;
+ else
+ freq = 24000000;
+ hw = clk_hw_register_fixed_rate(NULL, "clkin", NULL, 0, freq);
+ pr_debug("clkin @%u MHz\n", freq / 1000000);
+
+ /*
+ * High-speed PLL clock derived from the crystal. This the CPU clock,
+ * and we assume that it is enabled
+ */
+ regmap_read(map, ASPEED_HPLL_PARAM, &val);
+ aspeed_clk_data->hws[ASPEED_CLK_HPLL] = aspeed_ast2500_calc_pll("hpll", val);
+
+ /* Strap bits 11:9 define the AXI/AHB clock frequency ratio (aka HCLK)*/
+ regmap_read(map, ASPEED_STRAP, &val);
+ val = (val >> 9) & 0x7;
+ WARN(val == 0, "strapping is zero: cannot determine ahb clock");
+ div = 2 * (val + 1);
+ hw = clk_hw_register_fixed_factor(NULL, "ahb", "hpll", 0, 1, div);
+ aspeed_clk_data->hws[ASPEED_CLK_AHB] = hw;
+
+ /* APB clock clock selection register SCU08 (aka PCLK) */
+ regmap_read(map, ASPEED_CLK_SELECTION, &val);
+ val = (val >> 23) & 0x7;
+ div = 4 * (val + 1);
+ hw = clk_hw_register_fixed_factor(NULL, "apb", "hpll", 0, 1, div);
+ aspeed_clk_data->hws[ASPEED_CLK_APB] = hw;
+};
+
static void __init aspeed_cc_init(struct device_node *np)
{
struct regmap *map;
@@ -132,6 +302,13 @@ static void __init aspeed_cc_init(struct device_node *np)
return;
}
+ if (of_device_is_compatible(np, "aspeed,ast2400-scu"))
+ aspeed_ast2400_cc(map);
+ else if (of_device_is_compatible(np, "aspeed,ast2500-scu"))
+ aspeed_ast2500_cc(map);
+ else
+ pr_err("unknown platform, failed to add clocks\n");
+
aspeed_clk_data->num = ASPEED_NUM_CLKS;
ret = of_clk_add_hw_provider(np, of_clk_hw_onecell_get, aspeed_clk_data);
if (ret)
--
2.15.1
^ permalink raw reply related
* [PATCH v7 3/5] clk: aspeed: Add platform driver and register PLLs
From: Joel Stanley @ 2017-12-22 2:45 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171222024522.10362-1-joel@jms.id.au>
This registers a platform driver to set up all of the non-core clocks.
The clocks that have configurable rates are now registered.
Reviewed-by: Andrew Jeffery <andrew@aj.id.au>
Signed-off-by: Joel Stanley <joel@jms.id.au>
--
v6:
- Add Andrew's reviewed-by
v5:
- Remove eclk configuration. We do not have enough information to
correctly implement the mux and divisor, so it will have to be
implemented in the future
v4:
- Add eclk div table to fix ast2500 calculation
- Add defines to document the BIT() macros
- Pass dev where we can when registering clocks
- Check for errors when registering clk_hws
v3:
- Fix bclk and eclk calculation
- Separate out ast2400 and ast25000 for pll calculation
---
drivers/clk/clk-aspeed.c | 130 +++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 130 insertions(+)
diff --git a/drivers/clk/clk-aspeed.c b/drivers/clk/clk-aspeed.c
index 5adedda82d26..cf5ea63feb31 100644
--- a/drivers/clk/clk-aspeed.c
+++ b/drivers/clk/clk-aspeed.c
@@ -5,6 +5,8 @@
#include <linux/clk-provider.h>
#include <linux/mfd/syscon.h>
#include <linux/of_address.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
#include <linux/regmap.h>
#include <linux/slab.h>
#include <linux/spinlock.h>
@@ -107,6 +109,18 @@ static const struct aspeed_gate_data aspeed_gates[] = {
[ASPEED_CLK_GATE_LHCCLK] = { 28, -1, "lhclk-gate", "lhclk", 0 }, /* LPC master/LPC+ */
};
+static const struct clk_div_table ast2500_mac_div_table[] = {
+ { 0x0, 4 }, /* Yep, really. Aspeed confirmed this is correct */
+ { 0x1, 4 },
+ { 0x2, 6 },
+ { 0x3, 8 },
+ { 0x4, 10 },
+ { 0x5, 12 },
+ { 0x6, 14 },
+ { 0x7, 16 },
+ { 0 }
+};
+
static const struct clk_div_table ast2400_div_table[] = {
{ 0x0, 2 },
{ 0x1, 4 },
@@ -172,6 +186,122 @@ static struct clk_hw *aspeed_ast2500_calc_pll(const char *name, u32 val)
mult, div);
}
+struct aspeed_clk_soc_data {
+ const struct clk_div_table *div_table;
+ const struct clk_div_table *mac_div_table;
+ struct clk_hw *(*calc_pll)(const char *name, u32 val);
+};
+
+static const struct aspeed_clk_soc_data ast2500_data = {
+ .div_table = ast2500_div_table,
+ .mac_div_table = ast2500_mac_div_table,
+ .calc_pll = aspeed_ast2500_calc_pll,
+};
+
+static const struct aspeed_clk_soc_data ast2400_data = {
+ .div_table = ast2400_div_table,
+ .mac_div_table = ast2400_div_table,
+ .calc_pll = aspeed_ast2400_calc_pll,
+};
+
+static int aspeed_clk_probe(struct platform_device *pdev)
+{
+ const struct aspeed_clk_soc_data *soc_data;
+ struct device *dev = &pdev->dev;
+ struct regmap *map;
+ struct clk_hw *hw;
+ u32 val, rate;
+
+ map = syscon_node_to_regmap(dev->of_node);
+ if (IS_ERR(map)) {
+ dev_err(dev, "no syscon regmap\n");
+ return PTR_ERR(map);
+ }
+
+ /* SoC generations share common layouts but have different divisors */
+ soc_data = of_device_get_match_data(dev);
+ if (!soc_data) {
+ dev_err(dev, "no match data for platform\n");
+ return -EINVAL;
+ }
+
+ /* UART clock div13 setting */
+ regmap_read(map, ASPEED_MISC_CTRL, &val);
+ if (val & UART_DIV13_EN)
+ rate = 24000000 / 13;
+ else
+ rate = 24000000;
+ /* TODO: Find the parent data for the uart clock */
+ hw = clk_hw_register_fixed_rate(dev, "uart", NULL, 0, rate);
+ if (IS_ERR(hw))
+ return PTR_ERR(hw);
+ aspeed_clk_data->hws[ASPEED_CLK_UART] = hw;
+
+ /*
+ * Memory controller (M-PLL) PLL. This clock is configured by the
+ * bootloader, and is exposed to Linux as a read-only clock rate.
+ */
+ regmap_read(map, ASPEED_MPLL_PARAM, &val);
+ hw = soc_data->calc_pll("mpll", val);
+ if (IS_ERR(hw))
+ return PTR_ERR(hw);
+ aspeed_clk_data->hws[ASPEED_CLK_MPLL] = hw;
+
+ /* SD/SDIO clock divider (TODO: There's a gate too) */
+ hw = clk_hw_register_divider_table(dev, "sdio", "hpll", 0,
+ scu_base + ASPEED_CLK_SELECTION, 12, 3, 0,
+ soc_data->div_table,
+ &aspeed_clk_lock);
+ if (IS_ERR(hw))
+ return PTR_ERR(hw);
+ aspeed_clk_data->hws[ASPEED_CLK_SDIO] = hw;
+
+ /* MAC AHB bus clock divider */
+ hw = clk_hw_register_divider_table(dev, "mac", "hpll", 0,
+ scu_base + ASPEED_CLK_SELECTION, 16, 3, 0,
+ soc_data->mac_div_table,
+ &aspeed_clk_lock);
+ if (IS_ERR(hw))
+ return PTR_ERR(hw);
+ aspeed_clk_data->hws[ASPEED_CLK_MAC] = hw;
+
+ /* LPC Host (LHCLK) clock divider */
+ hw = clk_hw_register_divider_table(dev, "lhclk", "hpll", 0,
+ scu_base + ASPEED_CLK_SELECTION, 20, 3, 0,
+ soc_data->div_table,
+ &aspeed_clk_lock);
+ if (IS_ERR(hw))
+ return PTR_ERR(hw);
+ aspeed_clk_data->hws[ASPEED_CLK_LHCLK] = hw;
+
+ /* P-Bus (BCLK) clock divider */
+ hw = clk_hw_register_divider_table(dev, "bclk", "hpll", 0,
+ scu_base + ASPEED_CLK_SELECTION_2, 0, 2, 0,
+ soc_data->div_table,
+ &aspeed_clk_lock);
+ if (IS_ERR(hw))
+ return PTR_ERR(hw);
+ aspeed_clk_data->hws[ASPEED_CLK_BCLK] = hw;
+
+ return 0;
+};
+
+static const struct of_device_id aspeed_clk_dt_ids[] = {
+ { .compatible = "aspeed,ast2400-scu", .data = &ast2400_data },
+ { .compatible = "aspeed,ast2500-scu", .data = &ast2500_data },
+ { }
+};
+
+static struct platform_driver aspeed_clk_driver = {
+ .probe = aspeed_clk_probe,
+ .driver = {
+ .name = "aspeed-clk",
+ .of_match_table = aspeed_clk_dt_ids,
+ .suppress_bind_attrs = true,
+ },
+};
+builtin_platform_driver(aspeed_clk_driver);
+
static void __init aspeed_ast2400_cc(struct regmap *map)
{
struct clk_hw *hw;
--
2.15.1
^ permalink raw reply related
* [PATCH v7 4/5] clk: aspeed: Register gated clocks
From: Joel Stanley @ 2017-12-22 2:45 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171222024522.10362-1-joel@jms.id.au>
The majority of the clocks in the system are gates paired with a reset
controller that holds the IP in reset.
This borrows from clk_hw_register_gate, but registers two 'gates', one
to control the clock enable register and the other to control the reset
IP. This allows us to enforce the ordering:
1. Place IP in reset
2. Enable clock
3. Delay
4. Release reset
There are some gates that do not have an associated reset; these are
handled by using -1 as the index for the reset.
Reviewed-by: Andrew Jeffery <andrew@aj.id.au>
Signed-off-by: Joel Stanley <joel@jms.id.au>
---
v7:
- Use mdelay instead of msleep and remove the todo. Stephen points out:
> No you can't sleep here. It needs to delay because this is inside
> spinlock_irqsave.
v5:
- Add Andrew's Reviewed-by
v4:
- Drop useless 'disable clock' comment
- Drop CLK_IS_BASIC flag
- Fix 'there are a number of clocks...' comment
- Pass device to clk registration functions
- Check for errors when registering clk_hws
v3:
- Remove gates offset as gates are now at the start of the list
mdelay
Signed-off-by: Joel Stanley <joel@jms.id.au>
---
drivers/clk/clk-aspeed.c | 130 +++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 130 insertions(+)
diff --git a/drivers/clk/clk-aspeed.c b/drivers/clk/clk-aspeed.c
index cf5ea63feb31..dbd3c7774831 100644
--- a/drivers/clk/clk-aspeed.c
+++ b/drivers/clk/clk-aspeed.c
@@ -204,6 +204,106 @@ static const struct aspeed_clk_soc_data ast2400_data = {
.calc_pll = aspeed_ast2400_calc_pll,
};
+static int aspeed_clk_enable(struct clk_hw *hw)
+{
+ struct aspeed_clk_gate *gate = to_aspeed_clk_gate(hw);
+ unsigned long flags;
+ u32 clk = BIT(gate->clock_idx);
+ u32 rst = BIT(gate->reset_idx);
+
+ spin_lock_irqsave(gate->lock, flags);
+
+ if (gate->reset_idx >= 0) {
+ /* Put IP in reset */
+ regmap_update_bits(gate->map, ASPEED_RESET_CTRL, rst, rst);
+
+ /* Delay 100us */
+ udelay(100);
+ }
+
+ /* Enable clock */
+ regmap_update_bits(gate->map, ASPEED_CLK_STOP_CTRL, clk, 0);
+
+ if (gate->reset_idx >= 0) {
+ /* A delay of 10ms is specified by the ASPEED docs */
+ mdelay(10);
+
+ /* Take IP out of reset */
+ regmap_update_bits(gate->map, ASPEED_RESET_CTRL, rst, 0);
+ }
+
+ spin_unlock_irqrestore(gate->lock, flags);
+
+ return 0;
+}
+
+static void aspeed_clk_disable(struct clk_hw *hw)
+{
+ struct aspeed_clk_gate *gate = to_aspeed_clk_gate(hw);
+ unsigned long flags;
+ u32 clk = BIT(gate->clock_idx);
+
+ spin_lock_irqsave(gate->lock, flags);
+
+ regmap_update_bits(gate->map, ASPEED_CLK_STOP_CTRL, clk, clk);
+
+ spin_unlock_irqrestore(gate->lock, flags);
+}
+
+static int aspeed_clk_is_enabled(struct clk_hw *hw)
+{
+ struct aspeed_clk_gate *gate = to_aspeed_clk_gate(hw);
+ u32 clk = BIT(gate->clock_idx);
+ u32 reg;
+
+ regmap_read(gate->map, ASPEED_CLK_STOP_CTRL, ®);
+
+ return (reg & clk) ? 0 : 1;
+}
+
+static const struct clk_ops aspeed_clk_gate_ops = {
+ .enable = aspeed_clk_enable,
+ .disable = aspeed_clk_disable,
+ .is_enabled = aspeed_clk_is_enabled,
+};
+
+static struct clk_hw *aspeed_clk_hw_register_gate(struct device *dev,
+ const char *name, const char *parent_name, unsigned long flags,
+ struct regmap *map, u8 clock_idx, u8 reset_idx,
+ u8 clk_gate_flags, spinlock_t *lock)
+{
+ struct aspeed_clk_gate *gate;
+ struct clk_init_data init;
+ struct clk_hw *hw;
+ int ret;
+
+ gate = kzalloc(sizeof(*gate), GFP_KERNEL);
+ if (!gate)
+ return ERR_PTR(-ENOMEM);
+
+ init.name = name;
+ init.ops = &aspeed_clk_gate_ops;
+ init.flags = flags;
+ init.parent_names = parent_name ? &parent_name : NULL;
+ init.num_parents = parent_name ? 1 : 0;
+
+ gate->map = map;
+ gate->clock_idx = clock_idx;
+ gate->reset_idx = reset_idx;
+ gate->flags = clk_gate_flags;
+ gate->lock = lock;
+ gate->hw.init = &init;
+
+ hw = &gate->hw;
+ ret = clk_hw_register(dev, hw);
+ if (ret) {
+ kfree(gate);
+ hw = ERR_PTR(ret);
+ }
+
+ return hw;
+}
+
static int aspeed_clk_probe(struct platform_device *pdev)
{
const struct aspeed_clk_soc_data *soc_data;
@@ -211,6 +311,7 @@ static int aspeed_clk_probe(struct platform_device *pdev)
struct regmap *map;
struct clk_hw *hw;
u32 val, rate;
+ int i;
map = syscon_node_to_regmap(dev->of_node);
if (IS_ERR(map)) {
@@ -283,6 +384,35 @@ static int aspeed_clk_probe(struct platform_device *pdev)
return PTR_ERR(hw);
aspeed_clk_data->hws[ASPEED_CLK_BCLK] = hw;
+ /*
+ * TODO: There are a number of clocks that not included in this driver
+ * as more information is required:
+ * D2-PLL
+ * D-PLL
+ * YCLK
+ * RGMII
+ * RMII
+ * UART[1..5] clock source mux
+ * Video Engine (ECLK) mux and clock divider
+ */
+
+ for (i = 0; i < ARRAY_SIZE(aspeed_gates); i++) {
+ const struct aspeed_gate_data *gd = &aspeed_gates[i];
+
+ hw = aspeed_clk_hw_register_gate(dev,
+ gd->name,
+ gd->parent_name,
+ gd->flags,
+ map,
+ gd->clock_idx,
+ gd->reset_idx,
+ CLK_GATE_SET_TO_DISABLE,
+ &aspeed_clk_lock);
+ if (IS_ERR(hw))
+ return PTR_ERR(hw);
+ aspeed_clk_data->hws[i] = hw;
+ }
+
return 0;
};
--
2.15.1
^ permalink raw reply related
* [PATCH v7 5/5] clk: aspeed: Add reset controller
From: Joel Stanley @ 2017-12-22 2:45 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171222024522.10362-1-joel@jms.id.au>
There are some resets that are not associated with gates. These are
represented by a reset controller.
Reviewed-by: Andrew Jeffery <andrew@aj.id.au>
Signed-off-by: Joel Stanley <joel@jms.id.au>
---
v7:
- Rebase on dt-bindings patch
v5:
- Add Andrew's Reviewed-by
v3:
- Add named initalisers for the reset defines
- Add define for ADC
---
drivers/clk/clk-aspeed.c | 82 +++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 81 insertions(+), 1 deletion(-)
diff --git a/drivers/clk/clk-aspeed.c b/drivers/clk/clk-aspeed.c
index dbd3c7774831..6fb344730cea 100644
--- a/drivers/clk/clk-aspeed.c
+++ b/drivers/clk/clk-aspeed.c
@@ -8,6 +8,7 @@
#include <linux/of_device.h>
#include <linux/platform_device.h>
#include <linux/regmap.h>
+#include <linux/reset-controller.h>
#include <linux/slab.h>
#include <linux/spinlock.h>
@@ -267,6 +268,68 @@ static const struct clk_ops aspeed_clk_gate_ops = {
.is_enabled = aspeed_clk_is_enabled,
};
+/**
+ * struct aspeed_reset - Aspeed reset controller
+ * @map: regmap to access the containing system controller
+ * @rcdev: reset controller device
+ */
+struct aspeed_reset {
+ struct regmap *map;
+ struct reset_controller_dev rcdev;
+};
+
+#define to_aspeed_reset(p) container_of((p), struct aspeed_reset, rcdev)
+
+static const u8 aspeed_resets[] = {
+ [ASPEED_RESET_XDMA] = 25,
+ [ASPEED_RESET_MCTP] = 24,
+ [ASPEED_RESET_ADC] = 23,
+ [ASPEED_RESET_JTAG_MASTER] = 22,
+ [ASPEED_RESET_MIC] = 18,
+ [ASPEED_RESET_PWM] = 9,
+ [ASPEED_RESET_PCIVGA] = 8,
+ [ASPEED_RESET_I2C] = 2,
+ [ASPEED_RESET_AHB] = 1,
+};
+
+static int aspeed_reset_deassert(struct reset_controller_dev *rcdev,
+ unsigned long id)
+{
+ struct aspeed_reset *ar = to_aspeed_reset(rcdev);
+ u32 rst = BIT(aspeed_resets[id]);
+
+ return regmap_update_bits(ar->map, ASPEED_RESET_CTRL, rst, 0);
+}
+
+static int aspeed_reset_assert(struct reset_controller_dev *rcdev,
+ unsigned long id)
+{
+ struct aspeed_reset *ar = to_aspeed_reset(rcdev);
+ u32 rst = BIT(aspeed_resets[id]);
+
+ return regmap_update_bits(ar->map, ASPEED_RESET_CTRL, rst, rst);
+}
+
+static int aspeed_reset_status(struct reset_controller_dev *rcdev,
+ unsigned long id)
+{
+ struct aspeed_reset *ar = to_aspeed_reset(rcdev);
+ u32 val, rst = BIT(aspeed_resets[id]);
+ int ret;
+
+ ret = regmap_read(ar->map, ASPEED_RESET_CTRL, &val);
+ if (ret)
+ return ret;
+
+ return !!(val & rst);
+}
+
+static const struct reset_control_ops aspeed_reset_ops = {
+ .assert = aspeed_reset_assert,
+ .deassert = aspeed_reset_deassert,
+ .status = aspeed_reset_status,
+};
+
static struct clk_hw *aspeed_clk_hw_register_gate(struct device *dev,
const char *name, const char *parent_name, unsigned long flags,
struct regmap *map, u8 clock_idx, u8 reset_idx,
@@ -308,10 +371,11 @@ static int aspeed_clk_probe(struct platform_device *pdev)
{
const struct aspeed_clk_soc_data *soc_data;
struct device *dev = &pdev->dev;
+ struct aspeed_reset *ar;
struct regmap *map;
struct clk_hw *hw;
u32 val, rate;
- int i;
+ int i, ret;
map = syscon_node_to_regmap(dev->of_node);
if (IS_ERR(map)) {
@@ -319,6 +383,22 @@ static int aspeed_clk_probe(struct platform_device *pdev)
return PTR_ERR(map);
}
+ ar = devm_kzalloc(dev, sizeof(*ar), GFP_KERNEL);
+ if (!ar)
+ return -ENOMEM;
+
+ ar->map = map;
+ ar->rcdev.owner = THIS_MODULE;
+ ar->rcdev.nr_resets = ARRAY_SIZE(aspeed_resets);
+ ar->rcdev.ops = &aspeed_reset_ops;
+ ar->rcdev.of_node = dev->of_node;
+
+ ret = devm_reset_controller_register(dev, &ar->rcdev);
+ if (ret) {
+ dev_err(dev, "could not register reset controller\n");
+ return ret;
+ }
+
/* SoC generations share common layouts but have different divisors */
soc_data = of_device_get_match_data(dev);
if (!soc_data) {
--
2.15.1
^ permalink raw reply related
* [PATCH V4 1/1] clk: bulk: add of_clk_bulk_get()
From: Dong Aisheng @ 2017-12-22 2:51 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171221232032.GD7997@codeaurora.org>
On Thu, Dec 21, 2017 at 03:20:32PM -0800, Stephen Boyd wrote:
> On 12/20, Dong Aisheng wrote:
> > On Fri, Sep 29, 2017 at 03:48:21PM -0700, Stephen Boyd wrote:
> > > On 09/26, Dong Aisheng wrote:
> > > > here to handle this for DT users without 'clock-names' specified.
> >
> > > > +#endif
> > > >
> > > > void clk_bulk_put(int num_clks, struct clk_bulk_data *clks)
> > > > {
> > > > diff --git a/include/linux/clk.h b/include/linux/clk.h
> > > > index 12c96d9..073cb3b 100644
> > > > --- a/include/linux/clk.h
> > > > +++ b/include/linux/clk.h
> > > > @@ -680,10 +680,18 @@ static inline void clk_bulk_disable_unprepare(int num_clks,
> > > > }
> > > >
> > > > #if defined(CONFIG_OF) && defined(CONFIG_COMMON_CLK)
> > > > +int __must_check of_clk_bulk_get(struct device_node *np, int num_clks,
> > > > + struct clk_bulk_data *clks);
> > > > struct clk *of_clk_get(struct device_node *np, int index);
> > > > struct clk *of_clk_get_by_name(struct device_node *np, const char *name);
> > > > struct clk *of_clk_get_from_provider(struct of_phandle_args *clkspec);
> > > > #else
> > > > +static inline int of_clk_bulk_get(struct device_node *np, int num_clks,
> > >
> > > Do we need __must_check here too?
> >
> > Yes, you're absolutely right.
> >
> > of_clk_bulk_get is special as it returns error, so should add __must_check.
> >
> > > We should do the same for the
> > > other bulk get APIs. Seems we missed that part last time.
> > >
> >
> > Currently for !CONFIG_HAVE_CLK case, all APIs return 0.
> > !CONFIG_HAVE_CLK
> > clk_bulk_get return 0
> > devm_clk_bulk_get return 0
> > clk_bulk_enable return 0
> > clk_bulk_prepare return 0
> >
> > Do you think we still need add __must_check for them?
>
> Yes, we need it even when !CONFIG_HAVE_CLK because it allows us
> to catch missing checking return values in the non-clk compile
> configurations too. More test coverage.
>
Ok, understand.
May cook a patch to fix them.
> >
> > And for CONFIG_HAVE_CLK case, all __must_check already added.
> >
> > int __must_check clk_bulk_get
> > int __must_check devm_clk_bulk_get
> > int __must_check clk_bulk_enable
> > int __must_check clk_bulk_prepare
> >
> > And no need for void function.
> > void clk_bulk_put
> > void clk_bulk_unprepare
> > void clk_bulk_disable
> >
> > > I'll fix all these things up when applying.
> > >
> >
> > I did not see this in latest tree.
> > Suppose i should resend it with above things fixed, right?
> >
>
> I dropped it because it seems like maybe we don't need
> of_clk_bulk_get(), but more like clk_get_all() or something like
> that to acquire all clks for a device. It seems like it isn't DT
> specific, and so we should just provide the "all" API instead of
> some DT specific one that needs to know how many clks to get. I
> think I sent a similar reply on some other thread and added you
> to it.
>
I probably missed it before.
Will check later.
Thanks for reminder.
Regards
Dong Aisheng
^ permalink raw reply
* [PATCH V2 02/10] clk: reparent orphans after critical clocks enabled
From: Dong Aisheng @ 2017-12-22 2:54 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171222015507.GE7997@codeaurora.org>
On Thu, Dec 21, 2017 at 05:55:07PM -0800, Stephen Boyd wrote:
> On 12/21, Stephen Boyd wrote:
> >
> > Ok. Great. I'm going to apply this patch now into clk-next to
> > look for regressions on other platforms. If this was the only
> > questionable thing about this series then I think I can apply the
> > rest of it without you needing to resend. I'll check today.
> >
>
> I pushed it into clk-imx7ulp and merged that into clk-next. You
> can use that patch in your series.
>
Okay, great.
Thanks
Regards
Dong Aisheng
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox