* [PATCH] [RESEND] usb/gadget: aspeed-vhub: add USB_LIBCOMPOSITE dependency
From: Benjamin Herrenschmidt @ 2018-07-07 1:47 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20180706135840.908258-1-arnd@arndb.de>
On Fri, 2018-07-06 at 15:58 +0200, Arnd Bergmann wrote:
> Without that option, we run into a link failure:
>
> drivers/usb/gadget/udc/aspeed-vhub/hub.o: In function `ast_vhub_std_hub_request':
> hub.c:(.text+0x5b0): undefined reference to `usb_gadget_get_string'
>
> Fixes: 7ecca2a4080c ("usb/gadget: Add driver for Aspeed SoC virtual hub")
> Acked-by: Felipe Balbi <felipe.balbi@linux.intel.com>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> Greg, as discussed in https://patchwork.kernel.org/patch/10427921/,
> please apply this as a bugfix for 4.18.
Which I acked too though on second thoughts, shouldn't the driver
select USB_LIBCOMPOSITE ? No biggie, either way and that patch
can/should go in, we can change things later if we change our mind.
Cheers,
Ben.
> Arnd
> ---
> drivers/usb/gadget/udc/aspeed-vhub/Kconfig | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/usb/gadget/udc/aspeed-vhub/Kconfig b/drivers/usb/gadget/udc/aspeed-vhub/Kconfig
> index f0cdf89b8503..83ba8a2eb6af 100644
> --- a/drivers/usb/gadget/udc/aspeed-vhub/Kconfig
> +++ b/drivers/usb/gadget/udc/aspeed-vhub/Kconfig
> @@ -2,6 +2,7 @@
> config USB_ASPEED_VHUB
> tristate "Aspeed vHub UDC driver"
> depends on ARCH_ASPEED || COMPILE_TEST
> + depends on USB_LIBCOMPOSITE
> help
> USB peripheral controller for the Aspeed AST2500 family
> SoCs supporting the "vHub" functionality and USB2.0
^ permalink raw reply
* [PATCH] clk: aspeed: Fix SDCLK name
From: Stephen Boyd @ 2018-07-06 20:57 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <1529978125-28712-1-git-send-email-mine260309@gmail.com>
Quoting Lei YU (2018-06-25 18:55:25)
> The SDCLK was named SDCLKCLK, and no one has used this yet.
> Fix it.
>
> Signed-off-by: Lei YU <mine260309@gmail.com>
> ---
Applied to clk-next
^ permalink raw reply
* [PATCH] clk: aspeed: Mark bclk (PCIe) and dclk (VGA) as critical
From: Stephen Boyd @ 2018-07-06 20:48 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20180607070959.31995-1-joel@jms.id.au>
Quoting Joel Stanley (2018-06-07 00:09:59)
> This is used by the host to talk to the BMC's PCIe slave device. The BMC
> is not involved, but the clock needs to be enabled so the host can use
> the device.
>
> Fixes: 15ed8ce5f84e ("clk: aspeed: Register gated clocks")
> Cc: stable at vger.kernel.org # 4.15
> Acked-by: Andrew Jeffery <andrew@aj.id.au>
> Tested-by: Lei YU <mine260309@gmail.com>
> Signed-off-by: Joel Stanley <joel@jms.id.au>
> ---
Applied to clk-fixes
^ permalink raw reply
* [PATCH] drivers/misc: Aspeed LPC snoop output using misc chardev
From: Benjamin Fair @ 2018-07-06 18:25 UTC (permalink / raw)
To: linux-aspeed
From: Robert Lippert <rlippert@google.com>
Provides the data bytes snooped over the LPC snoop bus to userspace
as a (blocking) misc character device.
Bytes output from the host using LPC I/O transactions to the snooped port
can be watched or retrieved from the character device using a simple
command like this:
~# od -w1 -A n -t x1 /dev/aspeed-lpc-snoop0
10
de
ad
c0
ff
ee
Signed-off-by: Robert Lippert <rlippert@google.com>
Signed-off-by: Patrick Venture <venture@google.com>
Signed-off-by: Benjamin Fair <benjaminfair@google.com>
---
drivers/misc/aspeed-lpc-snoop.c | 84 +++++++++++++++++++++++++++++----
1 file changed, 74 insertions(+), 10 deletions(-)
diff --git a/drivers/misc/aspeed-lpc-snoop.c b/drivers/misc/aspeed-lpc-snoop.c
index cb78c98bc78d..2feb4347d67f 100644
--- a/drivers/misc/aspeed-lpc-snoop.c
+++ b/drivers/misc/aspeed-lpc-snoop.c
@@ -16,12 +16,15 @@
#include <linux/bitops.h>
#include <linux/interrupt.h>
+#include <linux/fs.h>
#include <linux/kfifo.h>
#include <linux/mfd/syscon.h>
+#include <linux/miscdevice.h>
#include <linux/module.h>
#include <linux/of.h>
#include <linux/of_device.h>
#include <linux/platform_device.h>
+#include <linux/poll.h>
#include <linux/regmap.h>
#define DEVICE_NAME "aspeed-lpc-snoop"
@@ -59,20 +62,70 @@ struct aspeed_lpc_snoop_model_data {
unsigned int has_hicrb_ensnp;
};
+struct aspeed_lpc_snoop_channel {
+ struct kfifo fifo;
+ wait_queue_head_t wq;
+ struct miscdevice miscdev;
+};
+
struct aspeed_lpc_snoop {
struct regmap *regmap;
int irq;
- struct kfifo snoop_fifo[NUM_SNOOP_CHANNELS];
+ struct aspeed_lpc_snoop_channel chan[NUM_SNOOP_CHANNELS];
+};
+
+static struct aspeed_lpc_snoop_channel *snoop_file_to_chan(struct file *file)
+{
+ return container_of(file->private_data,
+ struct aspeed_lpc_snoop_channel,
+ miscdev);
+}
+
+static ssize_t snoop_file_read(struct file *file, char __user *buffer,
+ size_t count, loff_t *ppos)
+{
+ struct aspeed_lpc_snoop_channel *chan = snoop_file_to_chan(file);
+ unsigned int copied;
+ int ret = 0;
+
+ if (kfifo_is_empty(&chan->fifo)) {
+ if (file->f_flags & O_NONBLOCK)
+ return -EAGAIN;
+ ret = wait_event_interruptible(chan->wq,
+ !kfifo_is_empty(&chan->fifo));
+ if (ret == -ERESTARTSYS)
+ return -EINTR;
+ }
+ ret = kfifo_to_user(&chan->fifo, buffer, count, &copied);
+
+ return ret ? ret : copied;
+}
+
+static unsigned int snoop_file_poll(struct file *file,
+ struct poll_table_struct *pt)
+{
+ struct aspeed_lpc_snoop_channel *chan = snoop_file_to_chan(file);
+
+ poll_wait(file, &chan->wq, pt);
+ return !kfifo_is_empty(&chan->fifo) ? POLLIN : 0;
+}
+
+static const struct file_operations snoop_fops = {
+ .owner = THIS_MODULE,
+ .read = snoop_file_read,
+ .poll = snoop_file_poll,
+ .llseek = noop_llseek,
};
/* Save a byte to a FIFO and discard the oldest byte if FIFO is full */
-static void put_fifo_with_discard(struct kfifo *fifo, u8 val)
+static void put_fifo_with_discard(struct aspeed_lpc_snoop_channel *chan, u8 val)
{
- if (!kfifo_initialized(fifo))
+ if (!kfifo_initialized(&chan->fifo))
return;
- if (kfifo_is_full(fifo))
- kfifo_skip(fifo);
- kfifo_put(fifo, val);
+ if (kfifo_is_full(&chan->fifo))
+ kfifo_skip(&chan->fifo);
+ kfifo_put(&chan->fifo, val);
+ wake_up_interruptible(&chan->wq);
}
static irqreturn_t aspeed_lpc_snoop_irq(int irq, void *arg)
@@ -97,12 +150,12 @@ static irqreturn_t aspeed_lpc_snoop_irq(int irq, void *arg)
if (reg & HICR6_STR_SNP0W) {
u8 val = (data & SNPWDR_CH0_MASK) >> SNPWDR_CH0_SHIFT;
- put_fifo_with_discard(&lpc_snoop->snoop_fifo[0], val);
+ put_fifo_with_discard(&lpc_snoop->chan[0], val);
}
if (reg & HICR6_STR_SNP1W) {
u8 val = (data & SNPWDR_CH1_MASK) >> SNPWDR_CH1_SHIFT;
- put_fifo_with_discard(&lpc_snoop->snoop_fifo[1], val);
+ put_fifo_with_discard(&lpc_snoop->chan[1], val);
}
return IRQ_HANDLED;
@@ -139,12 +192,22 @@ static int aspeed_lpc_enable_snoop(struct aspeed_lpc_snoop *lpc_snoop,
const struct aspeed_lpc_snoop_model_data *model_data =
of_device_get_match_data(dev);
+ init_waitqueue_head(&lpc_snoop->chan[channel].wq);
/* Create FIFO datastructure */
- rc = kfifo_alloc(&lpc_snoop->snoop_fifo[channel],
+ rc = kfifo_alloc(&lpc_snoop->chan[channel].fifo,
SNOOP_FIFO_SIZE, GFP_KERNEL);
if (rc)
return rc;
+ lpc_snoop->chan[channel].miscdev.minor = MISC_DYNAMIC_MINOR;
+ lpc_snoop->chan[channel].miscdev.name =
+ devm_kasprintf(dev, GFP_KERNEL, "%s%d", DEVICE_NAME, channel);
+ lpc_snoop->chan[channel].miscdev.fops = &snoop_fops;
+ lpc_snoop->chan[channel].miscdev.parent = dev;
+ rc = misc_register(&lpc_snoop->chan[channel].miscdev);
+ if (rc)
+ return rc;
+
/* Enable LPC snoop channel at requested port */
switch (channel) {
case 0:
@@ -191,7 +254,8 @@ static void aspeed_lpc_disable_snoop(struct aspeed_lpc_snoop *lpc_snoop,
return;
}
- kfifo_free(&lpc_snoop->snoop_fifo[channel]);
+ kfifo_free(&lpc_snoop->chan[channel].fifo);
+ misc_deregister(&lpc_snoop->chan[channel].miscdev);
}
static int aspeed_lpc_snoop_probe(struct platform_device *pdev)
--
2.18.0.203.gfac676dfb9-goog
^ permalink raw reply related
* [PATCH] clk: aspeed: Support HPLL strapping on ast2400
From: Stephen Boyd @ 2018-07-06 17:55 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20180628231540.26633-1-joel@jms.id.au>
Quoting Joel Stanley (2018-06-28 16:15:40)
> The HPLL can be configured through a register (SCU24), however some
> platforms chose to configure it through the strapping settings and do
> not use the register. This was not noticed as the logic for bit 18 in
> SCU24 was confused: set means programmed, but the driver read it as set
> means strapped.
>
> This gives us the correct HPLL value on Palmetto systems, from which
> most of the peripheral clocks are generated.
>
> Fixes: 5eda5d79e4be ("clk: Add clock driver for ASPEED BMC SoCs")
> Cc: stable at vger.kernel.org # v4.15
> Reviewed-by: C?dric Le Goater <clg@kaod.org>
> Signed-off-by: Joel Stanley <joel@jms.id.au>
> ---
Do you want this merged for -rc5? It sounds like on some systems this is
a problem, but I don't know if these systems are supposed to work yet or
not, so priority of this fix is not easy for me to understand.
^ permalink raw reply
* [PATCH] clk: aspeed: Treat a gate in reset as disabled
From: Stephen Boyd @ 2018-07-06 17:53 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <c31c3652829e8e7064f750d219237c4eb32773e4.camel@linux.vnet.ibm.com>
Quoting Benjamin Herrenschmidt (2018-07-03 00:24:47)
> On some systems, we come out of the bootloader with some
> gates set with the clock "enabled" but the reset also
> asserted.
>
> Since 8a53fc511c5e "clk: aspeed: Prevent reset if clock is enabled"
> we check that enabled bit in aspeed_clk_enabled(), and do
> nothing if already set.
>
> This breaks when the above scenario occurs, as the clock
> is enabled, but the reset still needs to be lifted.
>
> This patch fixes it by also checking the reset bit (if any)
> and treating a gate in "reset" as being disabled.
>
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Fixes: 8a53fc511c5e "clk: aspeed: Prevent reset if clock is enabled"
> CC: Eddie James <eajames@linux.vnet.ibm.com>
> ---
Applied to clk-fixes
^ permalink raw reply
* [PATCH] [RESEND] usb/gadget: aspeed-vhub: add USB_LIBCOMPOSITE dependency
From: Arnd Bergmann @ 2018-07-06 13:58 UTC (permalink / raw)
To: linux-aspeed
Without that option, we run into a link failure:
drivers/usb/gadget/udc/aspeed-vhub/hub.o: In function `ast_vhub_std_hub_request':
hub.c:(.text+0x5b0): undefined reference to `usb_gadget_get_string'
Fixes: 7ecca2a4080c ("usb/gadget: Add driver for Aspeed SoC virtual hub")
Acked-by: Felipe Balbi <felipe.balbi@linux.intel.com>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
Greg, as discussed in https://patchwork.kernel.org/patch/10427921/,
please apply this as a bugfix for 4.18.
Arnd
---
drivers/usb/gadget/udc/aspeed-vhub/Kconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/usb/gadget/udc/aspeed-vhub/Kconfig b/drivers/usb/gadget/udc/aspeed-vhub/Kconfig
index f0cdf89b8503..83ba8a2eb6af 100644
--- a/drivers/usb/gadget/udc/aspeed-vhub/Kconfig
+++ b/drivers/usb/gadget/udc/aspeed-vhub/Kconfig
@@ -2,6 +2,7 @@
config USB_ASPEED_VHUB
tristate "Aspeed vHub UDC driver"
depends on ARCH_ASPEED || COMPILE_TEST
+ depends on USB_LIBCOMPOSITE
help
USB peripheral controller for the Aspeed AST2500 family
SoCs supporting the "vHub" functionality and USB2.0
--
2.9.0
^ permalink raw reply related
* [PATCH 1/2] dt-bindings: fsi: Add optional chip-id to CFAMs
From: Benjamin Herrenschmidt @ 2018-07-06 1:48 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <CAL_JsqKZunLyRJQJgQrcBvv2s=HEd=E2ZG1ZgkgwTQkzJ88mcA@mail.gmail.com>
On Thu, 2018-07-05 at 12:49 -0600, Rob Herring wrote:
> On Tue, Jul 3, 2018 at 7:07 PM Benjamin Herrenschmidt
> <benh@kernel.crashing.org> wrote:
> >
> > On Tue, 2018-07-03 at 13:30 -0600, Rob Herring wrote:
> > > On Fri, Jun 22, 2018 at 02:37:55PM +1000, Benjamin Herrenschmidt wrote:
> > > > This represents a physical chip in the system and allows
> > > > a stable numbering scheme to be passed to udev for userspace
> > > > to recognize which chip is which.
> > >
> > > I'm sure you're aware, stable numbers is generally not something the
> > > kernel guarantees...
> >
> > This has nothing to do with any kernel guarantee. Not sure what you are
> > mixing up here :-)
>
> I was referring to /dev node names like sda/sdb/sdc or ttyS0/ttyS1.
Ah yes. I do have code to make those "somewhat stable" using the chip
ID by default (unless overriden) as a backward compat thing for small
systems but the long term goal is indeed to not rely on this.
> > The IDs will get exposed via sysfs in order to allow udev rules to
> > create appropriate symlinks such as by-id or by-path as is traditional
> > (we haven't completely decided some of the udev side details yet)
>
> Yeah, that's a bit different.
Yup, that's the right way actually :-)
> > > In the cases where we do have them, we've used aliases.
> >
> > This is necessary, though Aliases may do the job too. This is the
> > device-tree that represents the "host" system that the BMC is managing.
> >
> > We need to be able to identify using a stable numbering scheme the
> > processors on the FSI topology otherwise we would do "interesting"
> > things such as turn the fan for CPU 1 when CPU 0 gets hot :-)
> >
> > (This is just a silly example, there are plenty of other reasons why we
> > need to understand the HW topology of a given system, including
> > debuggers using FSI as a backend etc...)
> >
> > Traditionally POWER has used ibm,chip-id properties for the host side,
> > so I just did something similar here for the BMC side, but I can look
> > into using aliases if you prefer.
>
> That is specifically for CPUs though, right?
No, it's for host "chips".
IE, this is the device-tree of the BMC, consumed by the BMC. So we are
in the context of Linux running on the BMC ARM SoC.
That BMC is connected to the "Host" POWER9 chips via a service
interface called FSI (think as a kind of PECI or JTAG on steroids),
which it uses to crank them up or do various monitoring/management
tasks.
So while they are CPUs they are actually "Devices" in the context of
the consumer here.
Additionally, on some systems, there can be other type of chips
connected to FSI, such as memory buffer chips etc... all get a "chip
ID" assigned.
> Is the same true here? If
> so, I guess this is fine. A set of indexes for any device on a bus
> would be more concerning.
There can be multiple FSI links connecting to the same chips, some are
accessed via cascaded FSI masters etc... so it's not about indices on
the bus. It's really about identifying a physical package on the
motherboard.
> > Note: I'm not sure what you have against DT provided names or IDs, this
> > has been a rather standard way of doing things even before we did the
> > FDT. For example that's what slot-names properties are for, or location
> > codes etc... Yes we invented that alias trick later on but it's not
> > necessarily the best approach (in fact I don't really like it to be
> > honest).
>
> I'm no fan of aliases either, but just trying to keep some consistency
> in how we deal with various problems. My main concern is folks trying
> to create some made-up index numbering of devices. Often it's not
> really needed as there are other ways to address or identify devices.
> In many cases we end up using 'reg' if there's no other means of
> addressing a device.
>
> We've generally standardized around "label" for things like slots,
> ports, connectors, etc. that need to be physically identified.
Yes, label would be an option too, probably a better one that aliases.
> "slot-names" it seems hasn't gotten used for FDT. Since there aren't
> DT's published for OF based systems nor any documentation, newbies
> like me (that only have 8 years of DT experience) don't have any
> insight into how things used to be done.
In a pretty much ad-hoc way :-) In this case, though, chip-id is a
simple solution and works well (and I have the code already written and
tested :-)
Cheers,
Ben.
> Rob
^ permalink raw reply
* [PATCH] clk: aspeed: Mark bclk (PCIe) and dclk (VGA) as critical
From: Joel Stanley @ 2018-07-06 0:58 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20180607070959.31995-1-joel@jms.id.au>
On 7 June 2018 at 17:09, Joel Stanley <joel@jms.id.au> wrote:
> This is used by the host to talk to the BMC's PCIe slave device. The BMC
> is not involved, but the clock needs to be enabled so the host can use
> the device.
>
> Fixes: 15ed8ce5f84e ("clk: aspeed: Register gated clocks")
> Cc: stable at vger.kernel.org # 4.15
> Acked-by: Andrew Jeffery <andrew@aj.id.au>
> Tested-by: Lei YU <mine260309@gmail.com>
> Signed-off-by: Joel Stanley <joel@jms.id.au>
Ping?
> ---
> drivers/clk/clk-aspeed.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/clk/clk-aspeed.c b/drivers/clk/clk-aspeed.c
> index 4d425594999d..c17032bc853a 100644
> --- a/drivers/clk/clk-aspeed.c
> +++ b/drivers/clk/clk-aspeed.c
> @@ -91,8 +91,8 @@ static const struct aspeed_gate_data aspeed_gates[] = {
> [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, 8, "bclk-gate", "bclk", 0 }, /* PCIe/PCI */
> - [ASPEED_CLK_GATE_DCLK] = { 5, -1, "dclk-gate", NULL, 0 }, /* DAC */
> + [ASPEED_CLK_GATE_BCLK] = { 4, 8, "bclk-gate", "bclk", CLK_IS_CRITICAL }, /* PCIe/PCI */
> + [ASPEED_CLK_GATE_DCLK] = { 5, -1, "dclk-gate", NULL, CLK_IS_CRITICAL }, /* 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 */
> --
> 2.17.1
>
^ permalink raw reply
* [PATCH 1/2] dt-bindings: fsi: Add optional chip-id to CFAMs
From: Rob Herring @ 2018-07-05 18:49 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <23be30ebb2c661ef304c78a85cff591c515ba65b.camel@kernel.crashing.org>
On Tue, Jul 3, 2018 at 7:07 PM Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
>
> On Tue, 2018-07-03 at 13:30 -0600, Rob Herring wrote:
> > On Fri, Jun 22, 2018 at 02:37:55PM +1000, Benjamin Herrenschmidt wrote:
> > > This represents a physical chip in the system and allows
> > > a stable numbering scheme to be passed to udev for userspace
> > > to recognize which chip is which.
> >
> > I'm sure you're aware, stable numbers is generally not something the
> > kernel guarantees...
>
> This has nothing to do with any kernel guarantee. Not sure what you are
> mixing up here :-)
I was referring to /dev node names like sda/sdb/sdc or ttyS0/ttyS1.
> The IDs will get exposed via sysfs in order to allow udev rules to
> create appropriate symlinks such as by-id or by-path as is traditional
> (we haven't completely decided some of the udev side details yet)
Yeah, that's a bit different.
> > In the cases where we do have them, we've used aliases.
>
> This is necessary, though Aliases may do the job too. This is the
> device-tree that represents the "host" system that the BMC is managing.
>
> We need to be able to identify using a stable numbering scheme the
> processors on the FSI topology otherwise we would do "interesting"
> things such as turn the fan for CPU 1 when CPU 0 gets hot :-)
>
> (This is just a silly example, there are plenty of other reasons why we
> need to understand the HW topology of a given system, including
> debuggers using FSI as a backend etc...)
>
> Traditionally POWER has used ibm,chip-id properties for the host side,
> so I just did something similar here for the BMC side, but I can look
> into using aliases if you prefer.
That is specifically for CPUs though, right? Is the same true here? If
so, I guess this is fine. A set of indexes for any device on a bus
would be more concerning.
> Note: I'm not sure what you have against DT provided names or IDs, this
> has been a rather standard way of doing things even before we did the
> FDT. For example that's what slot-names properties are for, or location
> codes etc... Yes we invented that alias trick later on but it's not
> necessarily the best approach (in fact I don't really like it to be
> honest).
I'm no fan of aliases either, but just trying to keep some consistency
in how we deal with various problems. My main concern is folks trying
to create some made-up index numbering of devices. Often it's not
really needed as there are other ways to address or identify devices.
In many cases we end up using 'reg' if there's no other means of
addressing a device.
We've generally standardized around "label" for things like slots,
ports, connectors, etc. that need to be physically identified.
"slot-names" it seems hasn't gotten used for FDT. Since there aren't
DT's published for OF based systems nor any documentation, newbies
like me (that only have 8 years of DT experience) don't have any
insight into how things used to be done.
Rob
^ permalink raw reply
* [PATCH 11/14] dt-bindings: fsi: Document binding for the fsi-master-ast-cf "device"
From: Rob Herring @ 2018-07-05 16:08 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <77039070d470d5d408e750218ddbccf9cb33a78e.camel@kernel.crashing.org>
On Tue, Jul 3, 2018 at 7:16 PM Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
>
> On Tue, 2018-07-03 at 16:30 -0600, Rob Herring wrote:
> > On Wed, Jun 27, 2018 at 09:26:02AM +1000, Benjamin Herrenschmidt wrote:
> > > This isn't per-se a real device, it's a pseudo-device that
> > > represents the use of the Aspeed built-in ColdFire to
> > > implement the FSI protocol by bitbanging the GPIOs instead
> > > of doing it from the ARM core.
> > >
> > > Thus it's a drop-in replacement for the existing
> > > fsi-master-gpio pseudo-device for use on systems for which
> > > a corresponding firmware file exists. It has most of the
> > > same properties, plus some more needed to operate the
> > > coprocessor.
> > >
> > > Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> > > ---
> > > .../bindings/fsi/fsi-master-ast-cf.txt | 36 +++++++++++++++++++
> > > 1 file changed, 36 insertions(+)
> > > create mode 100644 Documentation/devicetree/bindings/fsi/fsi-master-ast-cf.txt
> > >
> > > diff --git a/Documentation/devicetree/bindings/fsi/fsi-master-ast-cf.txt b/Documentation/devicetree/bindings/fsi/fsi-master-ast-cf.txt
> > > new file mode 100644
> > > index 000000000000..50913ae685cc
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/fsi/fsi-master-ast-cf.txt
> > > @@ -0,0 +1,36 @@
> > > +Device-tree bindings for ColdFire offloaded gpio-based FSI master driver
> > > +------------------------------------------------------------------------
> > > +
> > > +Required properties:
> > > + - compatible =
> > > + "fsi-master-ast-2400-cf" for an AST2400 based system
> > > + or
> > > + "fsi-master-ast-2500-cf" for an AST2500 based system
> >
> > <vendor>,<soc>-<block>
>
> It's not really a SOC block from a vendor, it's a pseudo-device in a
> way. The current one that doesn't use the coldfire offload is just
> compatible "fsi-master-gpio".
>
> I can add a vendor but what should it be ? aspeed because it runs on
> the aspeed SoCs only ? ibm because we wrote it and FSI is an IBM
> protocol ?
I would say aspeed as it is tied to their chip.
>
> <soc>-<block> doesn't make sense here though.
But you do already have <soc> in the compatible, but in a slightly
different form and position. And "cf" is the block.
So I'd propose: aspeed,ast2500-cf-fsi-master
>
> > > +
> > > + - clock-gpios = <gpio-descriptor>; : GPIO for FSI clock
> > > + - data-gpios = <gpio-descriptor>; : GPIO for FSI data signal
> > > + - enable-gpios = <gpio-descriptor>; : GPIO for enable signal
> > > + - trans-gpios = <gpio-descriptor>; : GPIO for voltage translator enable
> > > + - mux-gpios = <gpio-descriptor>; : GPIO for pin multiplexing with other
> >
> > So the gpio info is pased to the CF? Otherwise, what's the point of
> > having these in DT?
>
> In the original version you are looking at, they are not passed to the
> CF per-se but the driver does use aspeed GPIO specific APIs to
> configure them to be owned by the CF, so we need the references.
Okay.
> However, I've just reworked the ucode with a few tricks to avoid losing
> singificant performance, so that we can indeed pass them to the CF,
> thus avoiding the need for a per-system image, so the above are here to
> stay.
>
> > > + functions (eg, external FSI masters)
> > > + - memory-region = <phandle>; : Reference to the reserved memory for
> > > + the ColdFire. Must be 2M aligned on
> > > + AST2400 and 1M aligned on AST2500
> > > + - sram = <phandle>; : Reference to the SRAM node.
> > > + - cvic = <phandle>; : Reference to the CVIC node.
> >
> > Vendor prefixes.
>
> On what ? Why would an "sram" pointer have a vendor prefix ? Or a
> memory region pointer ?
memory-region is a standard property. sram and cvic are not, so should
have vendor prefixes. However, perhaps we should add a common "sram"
property to sram/sram.txt.
Rob
^ permalink raw reply
* [PATCH] clk: aspeed: Treat a gate in reset as disabled
From: Joel Stanley @ 2018-07-04 6:55 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <c31c3652829e8e7064f750d219237c4eb32773e4.camel@linux.vnet.ibm.com>
On 3 July 2018 at 17:24, Benjamin Herrenschmidt <benh@linux.vnet.ibm.com> wrote:
> On some systems, we come out of the bootloader with some
> gates set with the clock "enabled" but the reset also
> asserted.
>
> Since 8a53fc511c5e "clk: aspeed: Prevent reset if clock is enabled"
> we check that enabled bit in aspeed_clk_enabled(), and do
> nothing if already set.
>
> This breaks when the above scenario occurs, as the clock
> is enabled, but the reset still needs to be lifted.
>
> This patch fixes it by also checking the reset bit (if any)
> and treating a gate in "reset" as being disabled.
>
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Fixes: 8a53fc511c5e "clk: aspeed: Prevent reset if clock is enabled"
> CC: Eddie James <eajames@linux.vnet.ibm.com>
Reviewed-by: Joel Stanley <joel@jms.id.au>
Thanks Ben.
> ---
> drivers/clk/clk-aspeed.c | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
> diff --git a/drivers/clk/clk-aspeed.c b/drivers/clk/clk-aspeed.c
> index c17032bc853a..c555eac2c528 100644
> --- a/drivers/clk/clk-aspeed.c
> +++ b/drivers/clk/clk-aspeed.c
> @@ -212,9 +212,22 @@ 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 rst = BIT(gate->reset_idx);
> u32 enval = (gate->flags & CLK_GATE_SET_TO_DISABLE) ? 0 : clk;
> u32 reg;
>
> + /*
> + * If the IP is in reset, treat the clock as not enabled,
> + * this happens with some clocks such as the USB one when
> + * coming from cold reset. Without this, aspeed_clk_enable()
> + * will fail to lift the reset.
> + */
> + if (gate->reset_idx >= 0) {
> + regmap_read(gate->map, ASPEED_RESET_CTRL, ®);
> + if (reg & rst)
> + return 0;
> + }
> +
> regmap_read(gate->map, ASPEED_CLK_STOP_CTRL, ®);
>
> return ((reg & clk) == enval) ? 1 : 0;
>
^ permalink raw reply
* [PATCH 11/14] dt-bindings: fsi: Document binding for the fsi-master-ast-cf "device"
From: Benjamin Herrenschmidt @ 2018-07-04 1:16 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20180703223046.GA20184@rob-hp-laptop>
On Tue, 2018-07-03 at 16:30 -0600, Rob Herring wrote:
> On Wed, Jun 27, 2018 at 09:26:02AM +1000, Benjamin Herrenschmidt wrote:
> > This isn't per-se a real device, it's a pseudo-device that
> > represents the use of the Aspeed built-in ColdFire to
> > implement the FSI protocol by bitbanging the GPIOs instead
> > of doing it from the ARM core.
> >
> > Thus it's a drop-in replacement for the existing
> > fsi-master-gpio pseudo-device for use on systems for which
> > a corresponding firmware file exists. It has most of the
> > same properties, plus some more needed to operate the
> > coprocessor.
> >
> > Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> > ---
> > .../bindings/fsi/fsi-master-ast-cf.txt | 36 +++++++++++++++++++
> > 1 file changed, 36 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/fsi/fsi-master-ast-cf.txt
> >
> > diff --git a/Documentation/devicetree/bindings/fsi/fsi-master-ast-cf.txt b/Documentation/devicetree/bindings/fsi/fsi-master-ast-cf.txt
> > new file mode 100644
> > index 000000000000..50913ae685cc
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/fsi/fsi-master-ast-cf.txt
> > @@ -0,0 +1,36 @@
> > +Device-tree bindings for ColdFire offloaded gpio-based FSI master driver
> > +------------------------------------------------------------------------
> > +
> > +Required properties:
> > + - compatible =
> > + "fsi-master-ast-2400-cf" for an AST2400 based system
> > + or
> > + "fsi-master-ast-2500-cf" for an AST2500 based system
>
> <vendor>,<soc>-<block>
It's not really a SOC block from a vendor, it's a pseudo-device in a
way. The current one that doesn't use the coldfire offload is just
compatible "fsi-master-gpio".
I can add a vendor but what should it be ? aspeed because it runs on
the aspeed SoCs only ? ibm because we wrote it and FSI is an IBM
protocol ?
<soc>-<block> doesn't make sense here though.
> > +
> > + - clock-gpios = <gpio-descriptor>; : GPIO for FSI clock
> > + - data-gpios = <gpio-descriptor>; : GPIO for FSI data signal
> > + - enable-gpios = <gpio-descriptor>; : GPIO for enable signal
> > + - trans-gpios = <gpio-descriptor>; : GPIO for voltage translator enable
> > + - mux-gpios = <gpio-descriptor>; : GPIO for pin multiplexing with other
>
> So the gpio info is pased to the CF? Otherwise, what's the point of
> having these in DT?
In the original version you are looking at, they are not passed to the
CF per-se but the driver does use aspeed GPIO specific APIs to
configure them to be owned by the CF, so we need the references.
However, I've just reworked the ucode with a few tricks to avoid losing
singificant performance, so that we can indeed pass them to the CF,
thus avoiding the need for a per-system image, so the above are here to
stay.
> > + functions (eg, external FSI masters)
> > + - memory-region = <phandle>; : Reference to the reserved memory for
> > + the ColdFire. Must be 2M aligned on
> > + AST2400 and 1M aligned on AST2500
> > + - sram = <phandle>; : Reference to the SRAM node.
> > + - cvic = <phandle>; : Reference to the CVIC node.
>
> Vendor prefixes.
On what ? Why would an "sram" pointer have a vendor prefix ? Or a
memory region pointer ?
> > + - fw-name = <string>; : The name used to construct the firmware
> > + file name (cf-fsi-<name>.bin)
>
> firmware-name is used in some other places (and is the full filename).
It's gone in the latest version as there's a single FW file now for all
systems.
>
> > + - fw-platform-sig = <value>; : A signature value that must match the one
> > + contained in the firmware image. Known
> > + values are listed in the firmware interface
> > + file cf-fsi-fw.h
>
> Vendor prefix unless you think this could be common.
It's going away.
> > +Examples:
> > +
> > + fsi-master {
> > + compatible = "fsi-master-gpio", "fsi-master";
>
> Forget to update the example?
Indeed :)
> > + clock-gpios = <&gpio 0>;
> > + data-gpios = <&gpio 1>;
> > + enable-gpios = <&gpio 2>;
> > + trans-gpios = <&gpio 3>;
> > + mux-gpios = <&gpio 4>;
> > + }
> > --
> > 2.17.1
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe devicetree" in
> > the body of a message to majordomo at vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH 1/2] dt-bindings: fsi: Add optional chip-id to CFAMs
From: Benjamin Herrenschmidt @ 2018-07-04 1:06 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20180703193017.GA23230@rob-hp-laptop>
On Tue, 2018-07-03 at 13:30 -0600, Rob Herring wrote:
> On Fri, Jun 22, 2018 at 02:37:55PM +1000, Benjamin Herrenschmidt wrote:
> > This represents a physical chip in the system and allows
> > a stable numbering scheme to be passed to udev for userspace
> > to recognize which chip is which.
>
> I'm sure you're aware, stable numbers is generally not something the
> kernel guarantees...
This has nothing to do with any kernel guarantee. Not sure what you are
mixing up here :-)
The IDs will get exposed via sysfs in order to allow udev rules to
create appropriate symlinks such as by-id or by-path as is traditional
(we haven't completely decided some of the udev side details yet)
> In the cases where we do have them, we've used aliases.
This is necessary, though Aliases may do the job too. This is the
device-tree that represents the "host" system that the BMC is managing.
We need to be able to identify using a stable numbering scheme the
processors on the FSI topology otherwise we would do "interesting"
things such as turn the fan for CPU 1 when CPU 0 gets hot :-)
(This is just a silly example, there are plenty of other reasons why we
need to understand the HW topology of a given system, including
debuggers using FSI as a backend etc...)
Traditionally POWER has used ibm,chip-id properties for the host side,
so I just did something similar here for the BMC side, but I can look
into using aliases if you prefer.
Note: I'm not sure what you have against DT provided names or IDs, this
has been a rather standard way of doing things even before we did the
FDT. For example that's what slot-names properties are for, or location
codes etc... Yes we invented that alias trick later on but it's not
necessarily the best approach (in fact I don't really like it to be
honest).
Cheers,
Ben.
>
> > Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> > ---
> > Documentation/devicetree/bindings/fsi/fsi.txt | 5 +++++
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/fsi/fsi.txt b/Documentation/devicetree/bindings/fsi/fsi.txt
> > index ab516c673a4b..afb4eccab131 100644
> > --- a/Documentation/devicetree/bindings/fsi/fsi.txt
> > +++ b/Documentation/devicetree/bindings/fsi/fsi.txt
> > @@ -83,6 +83,10 @@ addresses and sizes in the slave address space:
> > #address-cells = <1>;
> > #size-cells = <1>;
> >
> > +Optionally, a slave can provide a global unique chip ID which is used to
> > +identify the physical location of the chip in a system specific way
> > +
> > + chip-id = <0>;
> >
> > FSI engines (devices)
> > ---------------------
> > @@ -125,6 +129,7 @@ device tree if no extra platform information is required.
> > reg = <0 0>;
> > #address-cells = <1>;
> > #size-cells = <1>;
> > + chip-id = <0>;
> >
> > /* FSI engine at 0xc00, using a single page. In this example,
> > * it's an I2C master controller, so subnodes describe the
> > --
> > 2.17.1
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe devicetree" in
> > the body of a message to majordomo at vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH 11/14] dt-bindings: fsi: Document binding for the fsi-master-ast-cf "device"
From: Rob Herring @ 2018-07-03 22:30 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20180626232605.13420-12-benh@kernel.crashing.org>
On Wed, Jun 27, 2018 at 09:26:02AM +1000, Benjamin Herrenschmidt wrote:
> This isn't per-se a real device, it's a pseudo-device that
> represents the use of the Aspeed built-in ColdFire to
> implement the FSI protocol by bitbanging the GPIOs instead
> of doing it from the ARM core.
>
> Thus it's a drop-in replacement for the existing
> fsi-master-gpio pseudo-device for use on systems for which
> a corresponding firmware file exists. It has most of the
> same properties, plus some more needed to operate the
> coprocessor.
>
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> ---
> .../bindings/fsi/fsi-master-ast-cf.txt | 36 +++++++++++++++++++
> 1 file changed, 36 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/fsi/fsi-master-ast-cf.txt
>
> diff --git a/Documentation/devicetree/bindings/fsi/fsi-master-ast-cf.txt b/Documentation/devicetree/bindings/fsi/fsi-master-ast-cf.txt
> new file mode 100644
> index 000000000000..50913ae685cc
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/fsi/fsi-master-ast-cf.txt
> @@ -0,0 +1,36 @@
> +Device-tree bindings for ColdFire offloaded gpio-based FSI master driver
> +------------------------------------------------------------------------
> +
> +Required properties:
> + - compatible =
> + "fsi-master-ast-2400-cf" for an AST2400 based system
> + or
> + "fsi-master-ast-2500-cf" for an AST2500 based system
<vendor>,<soc>-<block>
> +
> + - clock-gpios = <gpio-descriptor>; : GPIO for FSI clock
> + - data-gpios = <gpio-descriptor>; : GPIO for FSI data signal
> + - enable-gpios = <gpio-descriptor>; : GPIO for enable signal
> + - trans-gpios = <gpio-descriptor>; : GPIO for voltage translator enable
> + - mux-gpios = <gpio-descriptor>; : GPIO for pin multiplexing with other
So the gpio info is pased to the CF? Otherwise, what's the point of
having these in DT?
> + functions (eg, external FSI masters)
> + - memory-region = <phandle>; : Reference to the reserved memory for
> + the ColdFire. Must be 2M aligned on
> + AST2400 and 1M aligned on AST2500
> + - sram = <phandle>; : Reference to the SRAM node.
> + - cvic = <phandle>; : Reference to the CVIC node.
Vendor prefixes.
> + - fw-name = <string>; : The name used to construct the firmware
> + file name (cf-fsi-<name>.bin)
firmware-name is used in some other places (and is the full filename).
> + - fw-platform-sig = <value>; : A signature value that must match the one
> + contained in the firmware image. Known
> + values are listed in the firmware interface
> + file cf-fsi-fw.h
Vendor prefix unless you think this could be common.
> +Examples:
> +
> + fsi-master {
> + compatible = "fsi-master-gpio", "fsi-master";
Forget to update the example?
> + clock-gpios = <&gpio 0>;
> + data-gpios = <&gpio 1>;
> + enable-gpios = <&gpio 2>;
> + trans-gpios = <&gpio 3>;
> + mux-gpios = <&gpio 4>;
> + }
> --
> 2.17.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH] clk: aspeed: Fix SDCLK name
From: Rob Herring @ 2018-07-03 22:09 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <1529978125-28712-1-git-send-email-mine260309@gmail.com>
On Tue, Jun 26, 2018 at 09:55:25AM +0800, Lei YU wrote:
> The SDCLK was named SDCLKCLK, and no one has used this yet.
> Fix it.
>
> Signed-off-by: Lei YU <mine260309@gmail.com>
> ---
> drivers/clk/clk-aspeed.c | 2 +-
> include/dt-bindings/clock/aspeed-clock.h | 2 +-
Acked-by: Rob Herring <robh@kernel.org>
> 2 files changed, 2 insertions(+), 2 deletions(-)
^ permalink raw reply
* [PATCH 1/2] dt-bindings: fsi: Add optional chip-id to CFAMs
From: Rob Herring @ 2018-07-03 19:30 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20180622043756.21158-1-benh@kernel.crashing.org>
On Fri, Jun 22, 2018 at 02:37:55PM +1000, Benjamin Herrenschmidt wrote:
> This represents a physical chip in the system and allows
> a stable numbering scheme to be passed to udev for userspace
> to recognize which chip is which.
I'm sure you're aware, stable numbers is generally not something the
kernel guarantees...
In the cases where we do have them, we've used aliases.
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> ---
> Documentation/devicetree/bindings/fsi/fsi.txt | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/fsi/fsi.txt b/Documentation/devicetree/bindings/fsi/fsi.txt
> index ab516c673a4b..afb4eccab131 100644
> --- a/Documentation/devicetree/bindings/fsi/fsi.txt
> +++ b/Documentation/devicetree/bindings/fsi/fsi.txt
> @@ -83,6 +83,10 @@ addresses and sizes in the slave address space:
> #address-cells = <1>;
> #size-cells = <1>;
>
> +Optionally, a slave can provide a global unique chip ID which is used to
> +identify the physical location of the chip in a system specific way
> +
> + chip-id = <0>;
>
> FSI engines (devices)
> ---------------------
> @@ -125,6 +129,7 @@ device tree if no extra platform information is required.
> reg = <0 0>;
> #address-cells = <1>;
> #size-cells = <1>;
> + chip-id = <0>;
>
> /* FSI engine at 0xc00, using a single page. In this example,
> * it's an I2C master controller, so subnodes describe the
> --
> 2.17.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH linux-next v6 04/13] dt-bindings: Add a document of PECI adapter driver for ASPEED AST24xx/25xx SoCs
From: Jae Hyun Yoo @ 2018-07-03 17:01 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20180703165148.GA11628@rob-hp-laptop>
Hi Rob,
Thanks a lot for the review. Please check my inline answer.
On 7/3/2018 9:51 AM, Rob Herring wrote:
> On Thu, Jun 21, 2018 at 12:40:24PM -0700, Jae Hyun Yoo wrote:
>> This commit adds a dt-bindings document of PECI adapter driver for ASPEED
>> AST24xx/25xx SoCs.
>>
>> Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
>> Reviewed-by: Haiyue Wang <haiyue.wang@linux.intel.com>
>> Reviewed-by: James Feist <james.feist@linux.intel.com>
>> Reviewed-by: Vernon Mauery <vernon.mauery@linux.intel.com>
>> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>> Cc: Greg KH <gregkh@linuxfoundation.org>
>> Cc: Jason M Biils <jason.m.bills@linux.intel.com>
>> Cc: Milton Miller II <miltonm@us.ibm.com>
>> Cc: Pavel Machek <pavel@ucw.cz>
>> Cc: Robin Murphy <robin.murphy@arm.com>
>> Cc: Ryan Chen <ryan_chen@aspeedtech.com>
>> ---
>> .../devicetree/bindings/peci/peci-aspeed.txt | 57 +++++++++++++++++++
>> 1 file changed, 57 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/peci/peci-aspeed.txt
>>
>> diff --git a/Documentation/devicetree/bindings/peci/peci-aspeed.txt b/Documentation/devicetree/bindings/peci/peci-aspeed.txt
>> new file mode 100644
>> index 000000000000..8c35f905589d
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/peci/peci-aspeed.txt
>> @@ -0,0 +1,57 @@
>> +Device tree configuration for PECI buses on the AST24XX and AST25XX SoCs.
>> +
>> +Required properties:
>> +- compatible : Should be "aspeed,ast2400-peci" or "aspeed,ast2500-peci"
>> + - aspeed,ast2400-peci: ASPEED AST2400 family PECI
>> + controller
>> + - aspeed,ast2500-peci: ASPEED AST2500 family PECI
>> + controller
>
> Just this is sufficient:
>
> Should be one of:
> "aspeed,ast2400-peci"
> "aspeed,ast2500-peci"
>
> With that,
>
> Reviewed-by: Rob Herring <robh@kernel.org>
>
Thanks! I'll change it as you commented out.
Jae
>> +- reg : Should contain PECI controller registers location and
>> + length.
>> +- #address-cells : Should be <1> required to define a client address.
>> +- #size-cells : Should be <0> required to define a client address.
>> +- interrupts : Should contain PECI controller interrupt.
>> +- clocks : Should contain clock source for PECI controller. Should
>> + reference the external oscillator clock in the second
>> + cell.
>> +- resets : Should contain phandle to reset controller with the reset
>> + number in the second cell.
>> +- clock-frequency : Should contain the operation frequency of PECI controller
>> + in units of Hz.
>> + 187500 ~ 24000000
>> +
>> +Optional properties:
>> +- msg-timing : Message timing negotiation period. This value will
>> + determine the period of message timing negotiation to be
>> + issued by PECI controller. The unit of the programmed
>> + value is four times of PECI clock period.
>> + 0 ~ 255 (default: 1)
>> +- addr-timing : Address timing negotiation period. This value will
>> + determine the period of address timing negotiation to be
>> + issued by PECI controller. The unit of the programmed
>> + value is four times of PECI clock period.
>> + 0 ~ 255 (default: 1)
>> +- rd-sampling-point : Read sampling point selection. The whole period of a bit
>> + time will be divided into 16 time frames. This value will
>> + determine the time frame in which the controller will
>> + sample PECI signal for data read back. Usually in the
>> + middle of a bit time is the best.
>> + 0 ~ 15 (default: 8)
>> +- cmd-timeout-ms : Command timeout in units of ms.
>> + 1 ~ 60000 (default: 1000)
>> +
>> +Example:
>> + peci0: peci-bus at 0 {
>> + compatible = "aspeed,ast2500-peci";
>> + reg = <0x0 0x60>;
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + interrupts = <15>;
>> + clocks = <&syscon ASPEED_CLK_GATE_REFCLK>;
>> + resets = <&syscon ASPEED_RESET_PECI>;
>> + clock-frequency = <24000000>;
>> + msg-timing = <1>;
>> + addr-timing = <1>;
>> + rd-sampling-point = <8>;
>> + cmd-timeout-ms = <1000>;
>> + };
>> --
>> 2.17.1
>>
^ permalink raw reply
* [PATCH linux-next v6 04/13] dt-bindings: Add a document of PECI adapter driver for ASPEED AST24xx/25xx SoCs
From: Rob Herring @ 2018-07-03 16:51 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20180621194024.20771-1-jae.hyun.yoo@linux.intel.com>
On Thu, Jun 21, 2018 at 12:40:24PM -0700, Jae Hyun Yoo wrote:
> This commit adds a dt-bindings document of PECI adapter driver for ASPEED
> AST24xx/25xx SoCs.
>
> Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
> Reviewed-by: Haiyue Wang <haiyue.wang@linux.intel.com>
> Reviewed-by: James Feist <james.feist@linux.intel.com>
> Reviewed-by: Vernon Mauery <vernon.mauery@linux.intel.com>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Greg KH <gregkh@linuxfoundation.org>
> Cc: Jason M Biils <jason.m.bills@linux.intel.com>
> Cc: Milton Miller II <miltonm@us.ibm.com>
> Cc: Pavel Machek <pavel@ucw.cz>
> Cc: Robin Murphy <robin.murphy@arm.com>
> Cc: Ryan Chen <ryan_chen@aspeedtech.com>
> ---
> .../devicetree/bindings/peci/peci-aspeed.txt | 57 +++++++++++++++++++
> 1 file changed, 57 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/peci/peci-aspeed.txt
>
> diff --git a/Documentation/devicetree/bindings/peci/peci-aspeed.txt b/Documentation/devicetree/bindings/peci/peci-aspeed.txt
> new file mode 100644
> index 000000000000..8c35f905589d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/peci/peci-aspeed.txt
> @@ -0,0 +1,57 @@
> +Device tree configuration for PECI buses on the AST24XX and AST25XX SoCs.
> +
> +Required properties:
> +- compatible : Should be "aspeed,ast2400-peci" or "aspeed,ast2500-peci"
> + - aspeed,ast2400-peci: ASPEED AST2400 family PECI
> + controller
> + - aspeed,ast2500-peci: ASPEED AST2500 family PECI
> + controller
Just this is sufficient:
Should be one of:
"aspeed,ast2400-peci"
"aspeed,ast2500-peci"
With that,
Reviewed-by: Rob Herring <robh@kernel.org>
> +- reg : Should contain PECI controller registers location and
> + length.
> +- #address-cells : Should be <1> required to define a client address.
> +- #size-cells : Should be <0> required to define a client address.
> +- interrupts : Should contain PECI controller interrupt.
> +- clocks : Should contain clock source for PECI controller. Should
> + reference the external oscillator clock in the second
> + cell.
> +- resets : Should contain phandle to reset controller with the reset
> + number in the second cell.
> +- clock-frequency : Should contain the operation frequency of PECI controller
> + in units of Hz.
> + 187500 ~ 24000000
> +
> +Optional properties:
> +- msg-timing : Message timing negotiation period. This value will
> + determine the period of message timing negotiation to be
> + issued by PECI controller. The unit of the programmed
> + value is four times of PECI clock period.
> + 0 ~ 255 (default: 1)
> +- addr-timing : Address timing negotiation period. This value will
> + determine the period of address timing negotiation to be
> + issued by PECI controller. The unit of the programmed
> + value is four times of PECI clock period.
> + 0 ~ 255 (default: 1)
> +- rd-sampling-point : Read sampling point selection. The whole period of a bit
> + time will be divided into 16 time frames. This value will
> + determine the time frame in which the controller will
> + sample PECI signal for data read back. Usually in the
> + middle of a bit time is the best.
> + 0 ~ 15 (default: 8)
> +- cmd-timeout-ms : Command timeout in units of ms.
> + 1 ~ 60000 (default: 1000)
> +
> +Example:
> + peci0: peci-bus at 0 {
> + compatible = "aspeed,ast2500-peci";
> + reg = <0x0 0x60>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + interrupts = <15>;
> + clocks = <&syscon ASPEED_CLK_GATE_REFCLK>;
> + resets = <&syscon ASPEED_RESET_PECI>;
> + clock-frequency = <24000000>;
> + msg-timing = <1>;
> + addr-timing = <1>;
> + rd-sampling-point = <8>;
> + cmd-timeout-ms = <1000>;
> + };
> --
> 2.17.1
>
^ permalink raw reply
* [PATCH] clk: aspeed: Treat a gate in reset as disabled
From: Benjamin Herrenschmidt @ 2018-07-03 7:24 UTC (permalink / raw)
To: linux-aspeed
On some systems, we come out of the bootloader with some
gates set with the clock "enabled" but the reset also
asserted.
Since 8a53fc511c5e "clk: aspeed: Prevent reset if clock is enabled"
we check that enabled bit in aspeed_clk_enabled(), and do
nothing if already set.
This breaks when the above scenario occurs, as the clock
is enabled, but the reset still needs to be lifted.
This patch fixes it by also checking the reset bit (if any)
and treating a gate in "reset" as being disabled.
Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Fixes: 8a53fc511c5e "clk: aspeed: Prevent reset if clock is enabled"
CC: Eddie James <eajames@linux.vnet.ibm.com>
---
drivers/clk/clk-aspeed.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/drivers/clk/clk-aspeed.c b/drivers/clk/clk-aspeed.c
index c17032bc853a..c555eac2c528 100644
--- a/drivers/clk/clk-aspeed.c
+++ b/drivers/clk/clk-aspeed.c
@@ -212,9 +212,22 @@ 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 rst = BIT(gate->reset_idx);
u32 enval = (gate->flags & CLK_GATE_SET_TO_DISABLE) ? 0 : clk;
u32 reg;
+ /*
+ * If the IP is in reset, treat the clock as not enabled,
+ * this happens with some clocks such as the USB one when
+ * coming from cold reset. Without this, aspeed_clk_enable()
+ * will fail to lift the reset.
+ */
+ if (gate->reset_idx >= 0) {
+ regmap_read(gate->map, ASPEED_RESET_CTRL, ®);
+ if (reg & rst)
+ return 0;
+ }
+
regmap_read(gate->map, ASPEED_CLK_STOP_CTRL, ®);
return ((reg & clk) == enval) ? 1 : 0;
^ permalink raw reply related
* [PATCH v4 0/4] gpio: aspeed: Fixes and support for sharing with co-processor
From: Benjamin Herrenschmidt @ 2018-07-03 0:59 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <CACRpkda_Rx+MRS_QZRYjbFomPsAhO5t4ViipA3+uHoGMaJEfrw@mail.gmail.com>
On Mon, 2018-07-02 at 16:13 +0200, Linus Walleij wrote:
> On Fri, Jun 29, 2018 at 6:11 AM Benjamin Herrenschmidt
> <benh@kernel.crashing.org> wrote:
>
> > This series cleans up the register accessors a bit, adds missing ones,
> > fixes the access to the write latch and finally adds an interface
> > that a co-processor driver can use to change the owner of some of
> > the GPIO lines and arbitrate access to shared GPIO banks.
> >
> > v2: - Address Joel comments
> > v3: - Adds Reviewed-by tags
> > v4: - Document the reason for including gpiolob.h
> > - Expose GPIO offsets & bit number so coprocessor firmware doesn't
> > have to hard-code them (allows multi-system generic firwmare
> > that gets "configure" by the copro driver)
>
> I've applied the v4 to the "ib-apeed" driver and merged that for
> devel, I will push to linux-next once the build servers confirm
> they are happy as well.
>
> The immutable branch is here:
> https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git/log/?h=ib-aspeed
Thanks. I'll pull it into the fsi tree so I can start laying out some
of the dependent stuff on top.
Cheers,
Ben.
^ permalink raw reply
* [PATCH] i2c: aspeed: Adjust spinlock scope in the irq handler
From: Jae Hyun Yoo @ 2018-07-02 21:40 UTC (permalink / raw)
To: linux-aspeed
This patch adjusts spinlock scope to make it wrap the whole irq
handler using a single lock/unlock which covers both master and
slave handlers.
Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
---
drivers/i2c/busses/i2c-aspeed.c | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-aspeed.c
index 60e4d0e939a3..9f02aa959a3e 100644
--- a/drivers/i2c/busses/i2c-aspeed.c
+++ b/drivers/i2c/busses/i2c-aspeed.c
@@ -234,7 +234,6 @@ static bool aspeed_i2c_slave_irq(struct aspeed_i2c_bus *bus)
bool irq_handled = true;
u8 value;
- spin_lock(&bus->lock);
if (!slave) {
irq_handled = false;
goto out;
@@ -325,7 +324,6 @@ static bool aspeed_i2c_slave_irq(struct aspeed_i2c_bus *bus)
writel(status_ack, bus->base + ASPEED_I2C_INTR_STS_REG);
out:
- spin_unlock(&bus->lock);
return irq_handled;
}
#endif /* CONFIG_I2C_SLAVE */
@@ -389,7 +387,6 @@ static bool aspeed_i2c_master_irq(struct aspeed_i2c_bus *bus)
u8 recv_byte;
int ret;
- spin_lock(&bus->lock);
irq_status = readl(bus->base + ASPEED_I2C_INTR_STS_REG);
/* Ack all interrupt bits. */
writel(irq_status, bus->base + ASPEED_I2C_INTR_STS_REG);
@@ -547,22 +544,29 @@ static bool aspeed_i2c_master_irq(struct aspeed_i2c_bus *bus)
dev_err(bus->dev,
"irq handled != irq. expected 0x%08x, but was 0x%08x\n",
irq_status, status_ack);
- spin_unlock(&bus->lock);
return !!irq_status;
}
static irqreturn_t aspeed_i2c_bus_irq(int irq, void *dev_id)
{
struct aspeed_i2c_bus *bus = dev_id;
+ bool ret;
+
+ spin_lock(&bus->lock);
#if IS_ENABLED(CONFIG_I2C_SLAVE)
if (aspeed_i2c_slave_irq(bus)) {
dev_dbg(bus->dev, "irq handled by slave.\n");
- return IRQ_HANDLED;
+ ret = true;
+ goto out;
}
#endif /* CONFIG_I2C_SLAVE */
- return aspeed_i2c_master_irq(bus) ? IRQ_HANDLED : IRQ_NONE;
+ ret = aspeed_i2c_master_irq(bus);
+
+out:
+ spin_unlock(&bus->lock);
+ return ret ? IRQ_HANDLED : IRQ_NONE;
}
static int aspeed_i2c_master_xfer(struct i2c_adapter *adap,
--
2.17.1
^ permalink raw reply related
* [PATCH] i2c: aspeed: Fix initial values of master and slave state
From: Jae Hyun Yoo @ 2018-07-02 21:20 UTC (permalink / raw)
To: linux-aspeed
This patch changes the order of enum aspeed_i2c_master_state and
enum aspeed_i2c_slave_state defines to make their initial value to
ASPEED_I2C_MASTER_INACTIVE and ASPEED_I2C_SLAVE_STOP respectively.
In case of multi-master use, if a slave data comes ahead of the
first master xfer, master_state starts from an invalid state so
this change fixes the issue.
Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
---
drivers/i2c/busses/i2c-aspeed.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-aspeed.c
index 60e4d0e939a3..2714c7fbe7c9 100644
--- a/drivers/i2c/busses/i2c-aspeed.c
+++ b/drivers/i2c/busses/i2c-aspeed.c
@@ -111,22 +111,22 @@
#define ASPEED_I2CD_DEV_ADDR_MASK GENMASK(6, 0)
enum aspeed_i2c_master_state {
+ ASPEED_I2C_MASTER_INACTIVE,
ASPEED_I2C_MASTER_START,
ASPEED_I2C_MASTER_TX_FIRST,
ASPEED_I2C_MASTER_TX,
ASPEED_I2C_MASTER_RX_FIRST,
ASPEED_I2C_MASTER_RX,
ASPEED_I2C_MASTER_STOP,
- ASPEED_I2C_MASTER_INACTIVE,
};
enum aspeed_i2c_slave_state {
+ ASPEED_I2C_SLAVE_STOP,
ASPEED_I2C_SLAVE_START,
ASPEED_I2C_SLAVE_READ_REQUESTED,
ASPEED_I2C_SLAVE_READ_PROCESSED,
ASPEED_I2C_SLAVE_WRITE_REQUESTED,
ASPEED_I2C_SLAVE_WRITE_RECEIVED,
- ASPEED_I2C_SLAVE_STOP,
};
struct aspeed_i2c_bus {
--
2.17.1
^ permalink raw reply related
* [PATCH] i2c: aspeed: Add newline characters into message printings.
From: Jae Hyun Yoo @ 2018-07-02 21:13 UTC (permalink / raw)
To: linux-aspeed
There are some log printing without a newline character. This
patch adds the missing newline characters.
Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
---
drivers/i2c/busses/i2c-aspeed.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-aspeed.c
index 60e4d0e939a3..e3007c1c4ac5 100644
--- a/drivers/i2c/busses/i2c-aspeed.c
+++ b/drivers/i2c/busses/i2c-aspeed.c
@@ -407,7 +407,7 @@ static bool aspeed_i2c_master_irq(struct aspeed_i2c_bus *bus)
*/
ret = aspeed_i2c_is_irq_error(irq_status);
if (ret < 0) {
- dev_dbg(bus->dev, "received error interrupt: 0x%08x",
+ dev_dbg(bus->dev, "received error interrupt: 0x%08x\n",
irq_status);
bus->cmd_err = ret;
bus->master_state = ASPEED_I2C_MASTER_INACTIVE;
@@ -416,7 +416,7 @@ static bool aspeed_i2c_master_irq(struct aspeed_i2c_bus *bus)
/* We are in an invalid state; reset bus to a known state. */
if (!bus->msgs) {
- dev_err(bus->dev, "bus in unknown state");
+ dev_err(bus->dev, "bus in unknown state\n");
bus->cmd_err = -EIO;
if (bus->master_state != ASPEED_I2C_MASTER_STOP)
aspeed_i2c_do_stop(bus);
@@ -431,7 +431,7 @@ static bool aspeed_i2c_master_irq(struct aspeed_i2c_bus *bus)
*/
if (bus->master_state == ASPEED_I2C_MASTER_START) {
if (unlikely(!(irq_status & ASPEED_I2CD_INTR_TX_ACK))) {
- pr_devel("no slave present at %02x", msg->addr);
+ pr_devel("no slave present at %02x\n", msg->addr);
status_ack |= ASPEED_I2CD_INTR_TX_NAK;
bus->cmd_err = -ENXIO;
aspeed_i2c_do_stop(bus);
@@ -451,11 +451,11 @@ static bool aspeed_i2c_master_irq(struct aspeed_i2c_bus *bus)
switch (bus->master_state) {
case ASPEED_I2C_MASTER_TX:
if (unlikely(irq_status & ASPEED_I2CD_INTR_TX_NAK)) {
- dev_dbg(bus->dev, "slave NACKed TX");
+ dev_dbg(bus->dev, "slave NACKed TX\n");
status_ack |= ASPEED_I2CD_INTR_TX_NAK;
goto error_and_stop;
} else if (unlikely(!(irq_status & ASPEED_I2CD_INTR_TX_ACK))) {
- dev_err(bus->dev, "slave failed to ACK TX");
+ dev_err(bus->dev, "slave failed to ACK TX\n");
goto error_and_stop;
}
status_ack |= ASPEED_I2CD_INTR_TX_ACK;
@@ -478,7 +478,7 @@ static bool aspeed_i2c_master_irq(struct aspeed_i2c_bus *bus)
/* fallthrough intended */
case ASPEED_I2C_MASTER_RX:
if (unlikely(!(irq_status & ASPEED_I2CD_INTR_RX_DONE))) {
- dev_err(bus->dev, "master failed to RX");
+ dev_err(bus->dev, "master failed to RX\n");
goto error_and_stop;
}
status_ack |= ASPEED_I2CD_INTR_RX_DONE;
@@ -509,7 +509,7 @@ static bool aspeed_i2c_master_irq(struct aspeed_i2c_bus *bus)
goto out_no_complete;
case ASPEED_I2C_MASTER_STOP:
if (unlikely(!(irq_status & ASPEED_I2CD_INTR_NORMAL_STOP))) {
- dev_err(bus->dev, "master failed to STOP");
+ dev_err(bus->dev, "master failed to STOP\n");
bus->cmd_err = -EIO;
/* Do not STOP as we have already tried. */
} else {
@@ -520,7 +520,7 @@ static bool aspeed_i2c_master_irq(struct aspeed_i2c_bus *bus)
goto out_complete;
case ASPEED_I2C_MASTER_INACTIVE:
dev_err(bus->dev,
- "master received interrupt 0x%08x, but is inactive",
+ "master received interrupt 0x%08x, but is inactive\n",
irq_status);
bus->cmd_err = -EIO;
/* Do not STOP as we should be inactive. */
@@ -851,7 +851,7 @@ static int aspeed_i2c_probe_bus(struct platform_device *pdev)
bus->rst = devm_reset_control_get_shared(&pdev->dev, NULL);
if (IS_ERR(bus->rst)) {
dev_err(&pdev->dev,
- "missing or invalid reset controller device tree entry");
+ "missing or invalid reset controller device tree entry\n");
return PTR_ERR(bus->rst);
}
reset_control_deassert(bus->rst);
--
2.17.1
^ permalink raw reply related
* [PATCH v4 0/4] gpio: aspeed: Fixes and support for sharing with co-processor
From: Linus Walleij @ 2018-07-02 14:13 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20180629041119.5132-1-benh@kernel.crashing.org>
On Fri, Jun 29, 2018 at 6:11 AM Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
> This series cleans up the register accessors a bit, adds missing ones,
> fixes the access to the write latch and finally adds an interface
> that a co-processor driver can use to change the owner of some of
> the GPIO lines and arbitrate access to shared GPIO banks.
>
> v2: - Address Joel comments
> v3: - Adds Reviewed-by tags
> v4: - Document the reason for including gpiolob.h
> - Expose GPIO offsets & bit number so coprocessor firmware doesn't
> have to hard-code them (allows multi-system generic firwmare
> that gets "configure" by the copro driver)
I've applied the v4 to the "ib-apeed" driver and merged that for
devel, I will push to linux-next once the build servers confirm
they are happy as well.
The immutable branch is here:
https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git/log/?h=ib-aspeed
Yours,
Linus Walleij
^ 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