Re: [PATCH] ARM: aspeed: g5: Do not set sirq polarity
Hi Joel,
> A feature was added to the aspeed vuart driver to configure the vuart
> interrupt (sirq) polarity according to the LPC/eSPI strapping register.
>
> Systems that depend on a active low behaviour (sirq_polarity set to 0)
> such as OpenPower boxes also use LPC, so this relationship does not
> hold.
>
> The property was added for a Tyan S7106 system which is not supported
> in the kernel tree. Should this or other systems wish to use this
> feature of the driver they should add it to the machine specific device
> tree.
>
> Fixes: c791fc76bc72 ("arm: dts: aspeed: Add vuart
> aspeed,sirq-polarity-sense...")
> Cc: sta...@vger.kernel.org
> Signed-off-by: Joel Stanley
LGTM. I've tested this on the s2600st, which is strapped for eSPI. All
good there too, as expected.
Tested-by: Jeremy Kerr
and/or:
Reviewed-by: Jeremy Kerr
Cheers,
Jeremy
Re: [PATCH] powerpc/spufs: add CONFIG_COREDUMP dependency
Hi Arnd,
> The kernel test robot pointed out a slightly different error message
> after recent commit 5456ffdee666 ("powerpc/spufs: simplify spufs core
> dumping") to spufs for a configuration that never worked:
>
>powerpc64-linux-ld: arch/powerpc/platforms/cell/spufs/file.o: in
> function `.spufs_proxydma_info_dump':
> > > file.c:(.text+0x4c68): undefined reference to `.dump_emit'
>powerpc64-linux-ld: arch/powerpc/platforms/cell/spufs/file.o: in
> function `.spufs_dma_info_dump':
>file.c:(.text+0x4d70): undefined reference to `.dump_emit'
>powerpc64-linux-ld: arch/powerpc/platforms/cell/spufs/file.o: in
> function `.spufs_wbox_info_dump':
>file.c:(.text+0x4df4): undefined reference to `.dump_emit'
>
> Add a Kconfig dependency to prevent this from happening again.
>
> Reported-by: kernel test robot
> Signed-off-by: Arnd Bergmann
Looks good to me, thanks.
Acked-by: Jeremy Kerr
Cheers,
Jeremy
[PATCH] net: bmac: Fix read of MAC address from ROM
In bmac_get_station_address, We're reading two bytes at a time from ROM,
but we do that six times, resulting in 12 bytes of read & writes. This
means we will write off the end of the six-byte destination buffer.
This change fixes the for-loop to only read/write six bytes.
Based on a proposed fix from Finn Thain .
Signed-off-by: Jeremy Kerr
Reported-by: Stan Johnson
Tested-by: Stan Johnson
Reported-by: Finn Thain
---
drivers/net/ethernet/apple/bmac.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/apple/bmac.c
b/drivers/net/ethernet/apple/bmac.c
index a58185b1d8bf..3e3711b60d01 100644
--- a/drivers/net/ethernet/apple/bmac.c
+++ b/drivers/net/ethernet/apple/bmac.c
@@ -1182,7 +1182,7 @@ bmac_get_station_address(struct net_device *dev, unsigned
char *ea)
int i;
unsigned short data;
- for (i = 0; i < 6; i++)
+ for (i = 0; i < 3; i++)
{
reset_and_select_srom(dev);
data = read_srom(dev, i + EnetAddressOffset/2,
SROMAddressBits);
--
2.17.1
Re: [PATCH] net: bmac: Fix stack corruption panic in bmac_probe()
Hi Stan, > The new kernel compiled and booted with no errors, with these > STACKPROTECTOR options in .config (the last two revealed the bug): > > CONFIG_HAVE_STACKPROTECTOR=y > CONFIG_CC_HAS_STACKPROTECTOR_NONE=y > CONFIG_STACKPROTECTOR=y > CONFIG_STACKPROTECTOR_STRONG=y Brilliant, thanks for testing. I'll send a standalone patch to netdev. Cheers, Jeremy
Re: [PATCH] net: bmac: Fix stack corruption panic in bmac_probe()
Hi Finn,
> This fixes an old bug recently revealed by CONFIG_STACKPROTECTOR.
Good catch. I'm not sure about the fix though. That variable ('addr')
should be a ethernet hardware address; I'm surprised we're accessing
past the 6th byte. The culprit seems to be this, where 'ea' is the
address buffer:
static void
bmac_get_station_address(struct net_device *dev, unsigned char *ea)
{
int i;
unsigned short data;
for (i = 0; i < 6; i++)
{
reset_and_select_srom(dev);
data = read_srom(dev, i + EnetAddressOffset/2,
SROMAddressBits);
ea[2*i] = bitrev8(data & 0x0ff);
ea[2*i+1] = bitrev8((data >> 8) & 0x0ff);
}
}
- where it looks like the condition on that for-loop is wrong; we're
reading two bytes at a time there.
Can you try the attached patch?
Ben/Paul - any thoughts?
Cheers,
Jeremy
-
>From 141b20bcbdb3ad7c166b83b4ea61f3521d0a0679 Mon Sep 17 00:00:00 2001
From: Jeremy Kerr
Date: Mon, 18 May 2020 08:54:25 +0800
Subject: [PATCH] net: bmac: Fix read of MAC address from ROM
In bmac_get_station_address, We're reading two bytes at a time from ROM,
but we do that six times, resulting in 12 bytes of read & writes. This
means we will write off the end of the six-byte destination buffer.
This change fixes the for-loop to only read/write six bytes.
Based on a proposed fix from Finn Thain .
Signed-off-by: Jeremy Kerr
Reported-by: Stan Johnson
Reported-by: Finn Thain
---
drivers/net/ethernet/apple/bmac.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/apple/bmac.c
b/drivers/net/ethernet/apple/bmac.c
index a58185b1d8bf..3e3711b60d01 100644
--- a/drivers/net/ethernet/apple/bmac.c
+++ b/drivers/net/ethernet/apple/bmac.c
@@ -1182,7 +1182,7 @@ bmac_get_station_address(struct net_device *dev, unsigned
char *ea)
int i;
unsigned short data;
- for (i = 0; i < 6; i++)
+ for (i = 0; i < 3; i++)
{
reset_and_select_srom(dev);
data = read_srom(dev, i + EnetAddressOffset/2,
SROMAddressBits);
--
2.17.1
Re: [PATCH] powerpc/spufs: adjust list element pointer type
Hi Julia, > Other uses of >aff_list_head, eg in spufs_assert_affinity, indicate > that the list elements have type spu_context, not spu as used here. Change > the type of tmp accordingly. Looks good to me; we could even use ctx there, rather than the separate tmp variable. Reviewed-by: Jeremy Kerr Cheers, Jeremy
Re: [RFC PATCH] powerpc/spufs: fix copy_to_user while atomic
Hi Christoph, > And another one that should go on top of this one to address Al's other > compaint: Yeah, I was pondering that one. The access_ok() is kinda redundant, but it does avoid forcing a SPU context save on those errors. However, it's not like we really need to optimise for the case of invalid addresses from userspace. So, I'll include this change in the submission to Michael's tree. Arnd - let me know if you have any objections. Cheers, Jeremy
Re: [RFC PATCH] powerpc/spufs: fix copy_to_user while atomic
Hi Christoph, > FYI, these little hunks reduce the difference to my version, maybe > you can fold them in? Sure, no problem. How do you want to coordinate these? I can submit mine through mpe, but that may make it tricky to synchronise with your changes. Or, you can include this change in your series if you prefer. Cheers, Jeremy
[RFC PATCH] powerpc/spufs: fix copy_to_user while atomic
Currently, we may perform a copy_to_user (through
simple_read_from_buffer()) while holding a context's register_lock,
while accessing the context save area.
This change uses a temporary buffers for the context save area data,
which we then pass to simple_read_from_buffer.
Signed-off-by: Jeremy Kerr
---
Christoph - this fixes the copy_to_user while atomic, hopefully with
only minimal distruption to your series.
---
arch/powerpc/platforms/cell/spufs/file.c | 110 +++
1 file changed, 74 insertions(+), 36 deletions(-)
diff --git a/arch/powerpc/platforms/cell/spufs/file.c
b/arch/powerpc/platforms/cell/spufs/file.c
index c0f950a3f4e1..c62d77ddaf7d 100644
--- a/arch/powerpc/platforms/cell/spufs/file.c
+++ b/arch/powerpc/platforms/cell/spufs/file.c
@@ -1978,8 +1978,9 @@ static ssize_t __spufs_mbox_info_read(struct spu_context
*ctx,
static ssize_t spufs_mbox_info_read(struct file *file, char __user *buf,
size_t len, loff_t *pos)
{
- int ret;
struct spu_context *ctx = file->private_data;
+ u32 stat, data;
+ int ret;
if (!access_ok(buf, len))
return -EFAULT;
@@ -1988,11 +1989,16 @@ static ssize_t spufs_mbox_info_read(struct file *file,
char __user *buf,
if (ret)
return ret;
spin_lock(>csa.register_lock);
- ret = __spufs_mbox_info_read(ctx, buf, len, pos);
+ stat = ctx->csa.prob.mb_stat_R;
+ data = ctx->csa.prob.pu_mb_R;
spin_unlock(>csa.register_lock);
spu_release_saved(ctx);
- return ret;
+ /* EOF if there's no entry in the mbox */
+ if (!(stat & 0xff))
+ return 0;
+
+ return simple_read_from_buffer(buf, len, pos, , sizeof(data));
}
static const struct file_operations spufs_mbox_info_fops = {
@@ -2019,6 +2025,7 @@ static ssize_t spufs_ibox_info_read(struct file *file,
char __user *buf,
size_t len, loff_t *pos)
{
struct spu_context *ctx = file->private_data;
+ u32 stat, data;
int ret;
if (!access_ok(buf, len))
@@ -2028,11 +2035,16 @@ static ssize_t spufs_ibox_info_read(struct file *file,
char __user *buf,
if (ret)
return ret;
spin_lock(>csa.register_lock);
- ret = __spufs_ibox_info_read(ctx, buf, len, pos);
+ stat = ctx->csa.prob.mb_stat_R;
+ data = ctx->csa.priv2.puint_mb_R;
spin_unlock(>csa.register_lock);
spu_release_saved(ctx);
- return ret;
+ /* EOF if there's no entry in the ibox */
+ if (!(stat & 0xff))
+ return 0;
+
+ return simple_read_from_buffer(buf, len, pos, , sizeof(data));
}
static const struct file_operations spufs_ibox_info_fops = {
@@ -2041,6 +2053,11 @@ static const struct file_operations spufs_ibox_info_fops
= {
.llseek = generic_file_llseek,
};
+static size_t spufs_wbox_info_cnt(struct spu_context *ctx)
+{
+ return (4 - ((ctx->csa.prob.mb_stat_R & 0x00ff00) >> 8)) * sizeof(u32);
+}
+
static ssize_t __spufs_wbox_info_read(struct spu_context *ctx,
char __user *buf, size_t len, loff_t *pos)
{
@@ -2049,7 +2066,7 @@ static ssize_t __spufs_wbox_info_read(struct spu_context
*ctx,
u32 wbox_stat;
wbox_stat = ctx->csa.prob.mb_stat_R;
- cnt = 4 - ((wbox_stat & 0x00ff00) >> 8);
+ cnt = spufs_wbox_info_cnt(ctx);
for (i = 0; i < cnt; i++) {
data[i] = ctx->csa.spu_mailbox_data[i];
}
@@ -2062,7 +2079,8 @@ static ssize_t spufs_wbox_info_read(struct file *file,
char __user *buf,
size_t len, loff_t *pos)
{
struct spu_context *ctx = file->private_data;
- int ret;
+ u32 data[ARRAY_SIZE(ctx->csa.spu_mailbox_data)];
+ int ret, count;
if (!access_ok(buf, len))
return -EFAULT;
@@ -2071,11 +2089,13 @@ static ssize_t spufs_wbox_info_read(struct file *file,
char __user *buf,
if (ret)
return ret;
spin_lock(>csa.register_lock);
- ret = __spufs_wbox_info_read(ctx, buf, len, pos);
+ count = spufs_wbox_info_cnt(ctx);
+ memcpy(, >csa.spu_mailbox_data, sizeof(data));
spin_unlock(>csa.register_lock);
spu_release_saved(ctx);
- return ret;
+ return simple_read_from_buffer(buf, len, pos, ,
+ count * sizeof(u32));
}
static const struct file_operations spufs_wbox_info_fops = {
@@ -2084,20 +2104,19 @@ static const struct file_operations
spufs_wbox_info_fops = {
.llseek = generic_file_llseek,
};
-static ssize_t __spufs_dma_info_read(struct spu_context *ctx,
- char __user *buf, size_t len, loff_t *pos)
+static void ___spufs_dma_info_read(struct spu_context *ctx,
+ struct spu_dma_info *info)
Re: [PATCH] MAINTAINERS: Add FSI subsystem
Hi Joel, > The subsystem was merged some time ago but we did not have a > maintainers > entry. Acked-by: Jeremy Kerr Cheers, Jeremy
Re: [PATCH] [RESEND] spufs: use timespec64 for timestamps
Hi Arnd, > The switch log prints the tv_sec portion of timespec as a 32-bit > number, while overflows in 2106. It also uses the timespec type, > which is safe on 64-bit architectures, but deprecated because > it causes overflows in 2038 elsewhere. > > This changes it to timespec64 and printing a 64-bit number for > consistency. If we still have spufs in the tree in 2038 I'd be worried :) But good to keep things consistent. Acked-by: Jeremy Kerr <j...@ozlabs.org> Michael: want to take this directly through your tree? Cheers, Jeremy
Re: [PATCH] [RESEND] spufs: use timespec64 for timestamps
Hi Arnd, > The switch log prints the tv_sec portion of timespec as a 32-bit > number, while overflows in 2106. It also uses the timespec type, > which is safe on 64-bit architectures, but deprecated because > it causes overflows in 2038 elsewhere. > > This changes it to timespec64 and printing a 64-bit number for > consistency. If we still have spufs in the tree in 2038 I'd be worried :) But good to keep things consistent. Acked-by: Jeremy Kerr Michael: want to take this directly through your tree? Cheers, Jeremy
Re: [PATCH net-next] net/ncsi: Don't take any action on HNCDSC AEN
Hi Sam, > The current HNCDSC handler takes the status flag from the AEN packet and > will update or change the current channel based on this flag and the > current channel status. > > However the flag from the HNCDSC packet merely represents the host link > state. While the state of the host interface is potentially interesting > information it should not affect the state of the NCSI link. Indeed the > NCSI specification makes no mention of any recommended action related to > the host network controller driver state. Yep, sounds good to me. If the link status does change, we should see an separate AEN for that event. Acked-by: Jeremy Kerr <j...@ozlabs.org> However: we're looking at some behaviour of Broadcom NICs at the moment, where the phy will be reset on link change events. We'd want to make sure that we're not just seeing the HNCDSC events for that too. I'd suggest we also get an Ack from Brian (CCed) to make sure we're not breaking that case. Cheers, Jeremy
Re: [PATCH net-next] net/ncsi: Don't take any action on HNCDSC AEN
Hi Sam, > The current HNCDSC handler takes the status flag from the AEN packet and > will update or change the current channel based on this flag and the > current channel status. > > However the flag from the HNCDSC packet merely represents the host link > state. While the state of the host interface is potentially interesting > information it should not affect the state of the NCSI link. Indeed the > NCSI specification makes no mention of any recommended action related to > the host network controller driver state. Yep, sounds good to me. If the link status does change, we should see an separate AEN for that event. Acked-by: Jeremy Kerr However: we're looking at some behaviour of Broadcom NICs at the moment, where the phy will be reset on link change events. We'd want to make sure that we're not just seeing the HNCDSC events for that too. I'd suggest we also get an Ack from Brian (CCed) to make sure we're not breaking that case. Cheers, Jeremy
Re: [PATCH] Make FSI a menuconfig to ease disabling it all
Hi Vincent, > No need to get into the submenu to disable all FSI-related config entries Sounds reasonable to me. Acked-by: Jeremy Kerr <j...@ozlabs.org> Greg: do you want Joel (or me) to manage FSI patches, or would you prefer to take this directly? Cheers, Jeremy
Re: [PATCH] Make FSI a menuconfig to ease disabling it all
Hi Vincent, > No need to get into the submenu to disable all FSI-related config entries Sounds reasonable to me. Acked-by: Jeremy Kerr Greg: do you want Joel (or me) to manage FSI patches, or would you prefer to take this directly? Cheers, Jeremy
Re: [RFC v1 4/4] ipmi_bmc: bt-aspeed: port driver to IPMI BMC framework
Hi Brendan, > The driver was handling interaction with userspace on its own. This > patch changes it to use the functionality of the ipmi_bmc framework > instead. > > Note that this removes the ability for the BMC to set SMS_ATN by making > an ioctl. If this functionality is required, it can be added back in > with a later patch. As Chris has mentioned, we do use this actively at the moment, so I'd prefer if we could not drop the support for SMS_ATN. However, using a different interface should be fine, if that helps. Cheers, Jeremy
Re: [RFC v1 4/4] ipmi_bmc: bt-aspeed: port driver to IPMI BMC framework
Hi Brendan, > The driver was handling interaction with userspace on its own. This > patch changes it to use the functionality of the ipmi_bmc framework > instead. > > Note that this removes the ability for the BMC to set SMS_ATN by making > an ioctl. If this functionality is required, it can be added back in > with a later patch. As Chris has mentioned, we do use this actively at the moment, so I'd prefer if we could not drop the support for SMS_ATN. However, using a different interface should be fine, if that helps. Cheers, Jeremy
[PATCH RFC] tty: don't force !TTYB_NORMAL flip buffers if not required
In change acc0f67f30, we introduced flip buffers that skipped allocation of the flags buffer for characters received with TTY_NORMAL flags. However, the slow path of tty_insert_flip_char() calls tty_insert_flip_string_flags() (providing a flag buffer pointer), which forces the buffer code to allocate a !TTYB_NORMAL buffer. If we took the slow path due to running out of buffer space, rather than seeing !TTY_NORMAL flags, we've needlessly allocated a flags buffer. This change uses tty_insert_flip_string_fixed_flag instead, which will allocate TTYB_NORMAL buffers if flag == TTY_NORMAL. Since we're only inserting one character, it's fine for the flag to be "fixed". Signed-off-by: Jeremy Kerr <j...@ozlabs.org> CC: Peter Hurley <pe...@hurleysoftware.com> --- RFC: I'm certainly no expert on the tty layer, and perhaps there's a good reason to always allocate a flags buffer. However, this seems to relieve buffer pressure in my tests. --- include/linux/tty_flip.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/linux/tty_flip.h b/include/linux/tty_flip.h index c28dd52..15d03a1 100644 --- a/include/linux/tty_flip.h +++ b/include/linux/tty_flip.h @@ -26,7 +26,7 @@ static inline int tty_insert_flip_char(struct tty_port *port, *char_buf_ptr(tb, tb->used++) = ch; return 1; } - return tty_insert_flip_string_flags(port, , , 1); + return tty_insert_flip_string_fixed_flag(port, , flag, 1); } static inline int tty_insert_flip_string(struct tty_port *port, -- 2.7.4
[PATCH RFC] tty: don't force !TTYB_NORMAL flip buffers if not required
In change acc0f67f30, we introduced flip buffers that skipped allocation of the flags buffer for characters received with TTY_NORMAL flags. However, the slow path of tty_insert_flip_char() calls tty_insert_flip_string_flags() (providing a flag buffer pointer), which forces the buffer code to allocate a !TTYB_NORMAL buffer. If we took the slow path due to running out of buffer space, rather than seeing !TTY_NORMAL flags, we've needlessly allocated a flags buffer. This change uses tty_insert_flip_string_fixed_flag instead, which will allocate TTYB_NORMAL buffers if flag == TTY_NORMAL. Since we're only inserting one character, it's fine for the flag to be "fixed". Signed-off-by: Jeremy Kerr CC: Peter Hurley --- RFC: I'm certainly no expert on the tty layer, and perhaps there's a good reason to always allocate a flags buffer. However, this seems to relieve buffer pressure in my tests. --- include/linux/tty_flip.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/linux/tty_flip.h b/include/linux/tty_flip.h index c28dd52..15d03a1 100644 --- a/include/linux/tty_flip.h +++ b/include/linux/tty_flip.h @@ -26,7 +26,7 @@ static inline int tty_insert_flip_char(struct tty_port *port, *char_buf_ptr(tb, tb->used++) = ch; return 1; } - return tty_insert_flip_string_flags(port, , , 1); + return tty_insert_flip_string_fixed_flag(port, , flag, 1); } static inline int tty_insert_flip_string(struct tty_port *port, -- 2.7.4
Re: [PATCH v2 0/3] pwm: add pwm AO on meson gx
Hi Thierry, > I /think/ Jeremy Kerr (To'ed) would be a good person to contact about > this. > > Jeremy, anything you can do about this? OK, all sorted. I've updated Jerome's entry in the database to suit. Cheers, Jeremy
Re: [PATCH v2 0/3] pwm: add pwm AO on meson gx
Hi Thierry, > I /think/ Jeremy Kerr (To'ed) would be a good person to contact about > this. > > Jeremy, anything you can do about this? OK, all sorted. I've updated Jerome's entry in the database to suit. Cheers, Jeremy
Re: [PATCH] fsi: core: register with postcore_initcall
Hi Joel, > This fix registers the bus with postcore_initcall instead, to ensure > it is registered earlier on. Looks good to me. Acked-by: Jeremy Kerr <j...@ozlabs.org> Cheers, Jeremy
Re: [PATCH] fsi: core: register with postcore_initcall
Hi Joel, > This fix registers the bus with postcore_initcall instead, to ensure > it is registered earlier on. Looks good to me. Acked-by: Jeremy Kerr Cheers, Jeremy
Re: [PATCH 6/6] Documentation/devicetree: Add FSI-attached I2C master dt bindings
Hi Eddie, >> Those child nodes represent the downstream i2c buses, and so also >> contain the i2c slave devices, right? If so, you may want to document >> that, and/or add a simple device to that example (say, an EEPROM). > > Yes, good point, but the driver currently wouldn't do anything with that > device information. It doesn't keep a list of populated devices on the > bus or anything. Still worth adding them to the device tree? Surely the i2c core needs this to be able to find i2c slave devices on the bus though? [You'll need to set i2c_adapter->dev.of_node for this to work though, which I don't think you are with the current patch set] Cheers, Jeremy
Re: [PATCH 6/6] Documentation/devicetree: Add FSI-attached I2C master dt bindings
Hi Eddie, >> Those child nodes represent the downstream i2c buses, and so also >> contain the i2c slave devices, right? If so, you may want to document >> that, and/or add a simple device to that example (say, an EEPROM). > > Yes, good point, but the driver currently wouldn't do anything with that > device information. It doesn't keep a list of populated devices on the > bus or anything. Still worth adding them to the device tree? Surely the i2c core needs this to be able to find i2c slave devices on the bus though? [You'll need to set i2c_adapter->dev.of_node for this to work though, which I don't think you are with the current patch set] Cheers, Jeremy
Re: [PATCH 6/6] Documentation/devicetree: Add FSI-attached I2C master dt bindings
Hi Eddie,
> +Required properties:
> + - compatible = "ibm,i2cm-fsi";
> + - reg = < address size >; : The FSI CFAM address and address space
> + size.
> + - #address-cells = <1>; : Number of address cells in child nodes
> + - #size-cells = <0>;: Number of size cells in child
> nodes.
> + - child nodes : Nodes to describe ports off
> the I2C
> + master.
> +
> +Child node required properties:
> + - reg = < port number > : The port number on the I2C master.
> +
> +Examples:
> +
> +i2cm@1800 {
> +compatible = "ibm,i2cm-fsi";
> +reg = < 0x1800 0x400 >;
> +#address-cells = <1>;
> +#size-cells = <0>;
> +
> +port@0 {
> +reg = <0>;
> +};
> +
> +port@1 {
> +reg = <1>;
> +};
> +};
Those child nodes represent the downstream i2c buses, and so also
contain the i2c slave devices, right? If so, you may want to document
that, and/or add a simple device to that example (say, an EEPROM).
Cheers,
Jeremy
Re: [PATCH 6/6] Documentation/devicetree: Add FSI-attached I2C master dt bindings
Hi Eddie,
> +Required properties:
> + - compatible = "ibm,i2cm-fsi";
> + - reg = < address size >; : The FSI CFAM address and address space
> + size.
> + - #address-cells = <1>; : Number of address cells in child nodes
> + - #size-cells = <0>;: Number of size cells in child
> nodes.
> + - child nodes : Nodes to describe ports off
> the I2C
> + master.
> +
> +Child node required properties:
> + - reg = < port number > : The port number on the I2C master.
> +
> +Examples:
> +
> +i2cm@1800 {
> +compatible = "ibm,i2cm-fsi";
> +reg = < 0x1800 0x400 >;
> +#address-cells = <1>;
> +#size-cells = <0>;
> +
> +port@0 {
> +reg = <0>;
> +};
> +
> +port@1 {
> +reg = <1>;
> +};
> +};
Those child nodes represent the downstream i2c buses, and so also
contain the i2c slave devices, right? If so, you may want to document
that, and/or add a simple device to that example (say, an EEPROM).
Cheers,
Jeremy
[PATCH v2 3/3] fsi/master-gpio: Add external mode
This change introduces an 'external mode' for GPIO-based FSI masters,
allowing the clock and data lines to be driven by an external source.
For example, external mode is selected by a user when an external debug
device is attached to the FSI pins.
To do this, we need to set specific states for the trans, mux and enable
gpios, and prevent access to clk & data from the FSI core code (by
returning EBUSY).
External mode is controlled by a sysfs attribute, so add the relevent
information to Documentation/ABI/
Signed-off-by: Jeremy Kerr <j...@ozlabs.org>
Reviewed-by: Joel Stanley <j...@jms.id.au>
---
.../ABI/testing/sysfs-driver-fsi-master-gpio | 10 +++
drivers/fsi/fsi-master-gpio.c | 78 +-
2 files changed, 86 insertions(+), 2 deletions(-)
create mode 100644 Documentation/ABI/testing/sysfs-driver-fsi-master-gpio
diff --git a/Documentation/ABI/testing/sysfs-driver-fsi-master-gpio
b/Documentation/ABI/testing/sysfs-driver-fsi-master-gpio
new file mode 100644
index 000..9667bb4
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-driver-fsi-master-gpio
@@ -0,0 +1,10 @@
+What: /sys/bus/platform/devices/[..]/fsi-master-gpio/external_mode
+Date: June 2017
+KernelVersion: 4.12
+Contact:j...@ozlabs.org
+Description:
+Controls access arbitration for GPIO-based FSI master. A
+ value of 0 (the default) sets normal mode, where the
+ driver performs FSI bus transactions, 1 sets external mode,
+ where the FSI bus is driven externally (for example, by
+ a debug device).
diff --git a/drivers/fsi/fsi-master-gpio.c b/drivers/fsi/fsi-master-gpio.c
index a6d602e..b54c213 100644
--- a/drivers/fsi/fsi-master-gpio.c
+++ b/drivers/fsi/fsi-master-gpio.c
@@ -59,6 +59,7 @@ struct fsi_master_gpio {
struct gpio_desc*gpio_trans;/* Voltage translator */
struct gpio_desc*gpio_enable; /* FSI enable */
struct gpio_desc*gpio_mux; /* Mux control */
+ boolexternal_mode;
};
#define CREATE_TRACE_POINTS
@@ -411,6 +412,12 @@ static int fsi_master_gpio_xfer(struct fsi_master_gpio
*master, uint8_t slave,
int rc;
spin_lock_irqsave(>cmd_lock, flags);
+
+ if (master->external_mode) {
+ spin_unlock_irqrestore(>cmd_lock, flags);
+ return -EBUSY;
+ }
+
serial_out(master, cmd);
echo_delay(master);
rc = poll_for_response(master, slave, resp_len, resp);
@@ -469,6 +476,10 @@ static int fsi_master_gpio_break(struct fsi_master
*_master, int link)
trace_fsi_master_gpio_break(master);
spin_lock_irqsave(>cmd_lock, flags);
+ if (master->external_mode) {
+ spin_unlock_irqrestore(>cmd_lock, flags);
+ return -EBUSY;
+ }
set_sda_output(master, 1);
sda_out(master, 1);
clock_toggle(master, FSI_PRE_BREAK_CLOCKS);
@@ -497,25 +508,84 @@ static void fsi_master_gpio_init(struct fsi_master_gpio
*master)
clock_zeros(master, FSI_INIT_CLOCKS);
}
+static void fsi_master_gpio_init_external(struct fsi_master_gpio *master)
+{
+ gpiod_direction_output(master->gpio_mux, 0);
+ gpiod_direction_output(master->gpio_trans, 0);
+ gpiod_direction_output(master->gpio_enable, 1);
+ gpiod_direction_input(master->gpio_clk);
+ gpiod_direction_input(master->gpio_data);
+}
+
static int fsi_master_gpio_link_enable(struct fsi_master *_master, int link)
{
struct fsi_master_gpio *master = to_fsi_master_gpio(_master);
unsigned long flags;
+ int rc = -EBUSY;
if (link != 0)
return -ENODEV;
spin_lock_irqsave(>cmd_lock, flags);
- gpiod_set_value(master->gpio_enable, 1);
+ if (!master->external_mode) {
+ gpiod_set_value(master->gpio_enable, 1);
+ rc = 0;
+ }
spin_unlock_irqrestore(>cmd_lock, flags);
- return 0;
+ return rc;
+}
+
+static ssize_t external_mode_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct fsi_master_gpio *master = dev_get_drvdata(dev);
+
+ return snprintf(buf, PAGE_SIZE - 1, "%u\n",
+ master->external_mode ? 1 : 0);
+}
+
+static ssize_t external_mode_store(struct device *dev,
+ struct device_attribute *attr, const char *buf, size_t count)
+{
+ struct fsi_master_gpio *master = dev_get_drvdata(dev);
+ unsigned long flags, val;
+ bool external_mode;
+ int err;
+
+ err = kstrtoul(buf, 0, );
+ if (err)
+ return err;
+
+ external_mode = !!val;
+
+ spin_lock_irqsave(>cmd_lock, flags);
+
+ if (external_mode == master->external_mode) {
+ spin_unlock_irqrestore(>cmd
[PATCH v2 3/3] fsi/master-gpio: Add external mode
This change introduces an 'external mode' for GPIO-based FSI masters,
allowing the clock and data lines to be driven by an external source.
For example, external mode is selected by a user when an external debug
device is attached to the FSI pins.
To do this, we need to set specific states for the trans, mux and enable
gpios, and prevent access to clk & data from the FSI core code (by
returning EBUSY).
External mode is controlled by a sysfs attribute, so add the relevent
information to Documentation/ABI/
Signed-off-by: Jeremy Kerr
Reviewed-by: Joel Stanley
---
.../ABI/testing/sysfs-driver-fsi-master-gpio | 10 +++
drivers/fsi/fsi-master-gpio.c | 78 +-
2 files changed, 86 insertions(+), 2 deletions(-)
create mode 100644 Documentation/ABI/testing/sysfs-driver-fsi-master-gpio
diff --git a/Documentation/ABI/testing/sysfs-driver-fsi-master-gpio
b/Documentation/ABI/testing/sysfs-driver-fsi-master-gpio
new file mode 100644
index 000..9667bb4
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-driver-fsi-master-gpio
@@ -0,0 +1,10 @@
+What: /sys/bus/platform/devices/[..]/fsi-master-gpio/external_mode
+Date: June 2017
+KernelVersion: 4.12
+Contact:j...@ozlabs.org
+Description:
+Controls access arbitration for GPIO-based FSI master. A
+ value of 0 (the default) sets normal mode, where the
+ driver performs FSI bus transactions, 1 sets external mode,
+ where the FSI bus is driven externally (for example, by
+ a debug device).
diff --git a/drivers/fsi/fsi-master-gpio.c b/drivers/fsi/fsi-master-gpio.c
index a6d602e..b54c213 100644
--- a/drivers/fsi/fsi-master-gpio.c
+++ b/drivers/fsi/fsi-master-gpio.c
@@ -59,6 +59,7 @@ struct fsi_master_gpio {
struct gpio_desc*gpio_trans;/* Voltage translator */
struct gpio_desc*gpio_enable; /* FSI enable */
struct gpio_desc*gpio_mux; /* Mux control */
+ boolexternal_mode;
};
#define CREATE_TRACE_POINTS
@@ -411,6 +412,12 @@ static int fsi_master_gpio_xfer(struct fsi_master_gpio
*master, uint8_t slave,
int rc;
spin_lock_irqsave(>cmd_lock, flags);
+
+ if (master->external_mode) {
+ spin_unlock_irqrestore(>cmd_lock, flags);
+ return -EBUSY;
+ }
+
serial_out(master, cmd);
echo_delay(master);
rc = poll_for_response(master, slave, resp_len, resp);
@@ -469,6 +476,10 @@ static int fsi_master_gpio_break(struct fsi_master
*_master, int link)
trace_fsi_master_gpio_break(master);
spin_lock_irqsave(>cmd_lock, flags);
+ if (master->external_mode) {
+ spin_unlock_irqrestore(>cmd_lock, flags);
+ return -EBUSY;
+ }
set_sda_output(master, 1);
sda_out(master, 1);
clock_toggle(master, FSI_PRE_BREAK_CLOCKS);
@@ -497,25 +508,84 @@ static void fsi_master_gpio_init(struct fsi_master_gpio
*master)
clock_zeros(master, FSI_INIT_CLOCKS);
}
+static void fsi_master_gpio_init_external(struct fsi_master_gpio *master)
+{
+ gpiod_direction_output(master->gpio_mux, 0);
+ gpiod_direction_output(master->gpio_trans, 0);
+ gpiod_direction_output(master->gpio_enable, 1);
+ gpiod_direction_input(master->gpio_clk);
+ gpiod_direction_input(master->gpio_data);
+}
+
static int fsi_master_gpio_link_enable(struct fsi_master *_master, int link)
{
struct fsi_master_gpio *master = to_fsi_master_gpio(_master);
unsigned long flags;
+ int rc = -EBUSY;
if (link != 0)
return -ENODEV;
spin_lock_irqsave(>cmd_lock, flags);
- gpiod_set_value(master->gpio_enable, 1);
+ if (!master->external_mode) {
+ gpiod_set_value(master->gpio_enable, 1);
+ rc = 0;
+ }
spin_unlock_irqrestore(>cmd_lock, flags);
- return 0;
+ return rc;
+}
+
+static ssize_t external_mode_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct fsi_master_gpio *master = dev_get_drvdata(dev);
+
+ return snprintf(buf, PAGE_SIZE - 1, "%u\n",
+ master->external_mode ? 1 : 0);
+}
+
+static ssize_t external_mode_store(struct device *dev,
+ struct device_attribute *attr, const char *buf, size_t count)
+{
+ struct fsi_master_gpio *master = dev_get_drvdata(dev);
+ unsigned long flags, val;
+ bool external_mode;
+ int err;
+
+ err = kstrtoul(buf, 0, );
+ if (err)
+ return err;
+
+ external_mode = !!val;
+
+ spin_lock_irqsave(>cmd_lock, flags);
+
+ if (external_mode == master->external_mode) {
+ spin_unlock_irqrestore(>cmd_lock, flags);
+ return count;
+
Re: [PATCH 3/3] fsi/master-gpio: Add external mode
Hi Joel,
>> +static ssize_t external_mode_show(struct device *dev,
>> + struct device_attribute *attr, char *buf)
>> +{
>> + struct fsi_master_gpio *master = dev_get_drvdata(dev);
>> +
>> + return snprintf(buf, PAGE_SIZE - 1, "%u",
>
> I gave this a spin on a machine today and noticed we're missing a
> newline there. Should this be "%u\n"?
Yep, that'd make things easier to read. v2 coming.
Cheers,
Jeremy
Re: [PATCH 3/3] fsi/master-gpio: Add external mode
Hi Joel,
>> +static ssize_t external_mode_show(struct device *dev,
>> + struct device_attribute *attr, char *buf)
>> +{
>> + struct fsi_master_gpio *master = dev_get_drvdata(dev);
>> +
>> + return snprintf(buf, PAGE_SIZE - 1, "%u",
>
> I gave this a spin on a machine today and noticed we're missing a
> newline there. Should this be "%u\n"?
Yep, that'd make things easier to read. v2 coming.
Cheers,
Jeremy
Re: [PATCH] drivers/fsi: fix fsi_slave_mode prototype
Hi Arnd, > gcc warns about the return type of this function: > > drivers/fsi/fsi-core.c:535:8: error: type qualifiers ignored on function > return type [-Werror=ignored-qualifiers] > > This removes the 'const' attribute, as suggested by the warning. Acked-by: Jeremy Kerr <j...@ozlabs.org> Cheers, Jeremy
Re: [PATCH] drivers/fsi: fix fsi_slave_mode prototype
Hi Arnd, > gcc warns about the return type of this function: > > drivers/fsi/fsi-core.c:535:8: error: type qualifiers ignored on function > return type [-Werror=ignored-qualifiers] > > This removes the 'const' attribute, as suggested by the warning. Acked-by: Jeremy Kerr Cheers, Jeremy
[PATCH v2 1/3] fsi: Add fsi_master_rescan()
We'll want non-core fsi code to trigger a rescan, so introduce a
non-static fsi_master_rescan() function. Use this for the existing
unscan/scan behaviour too.
Signed-off-by: Jeremy Kerr <j...@ozlabs.org>
Reviewed-by: Joel Stanley <j...@jms.id.au>
Reviewed-by: Christopher Bostic <clbos...@linux.vnet.ibm.com>
---
v2:
- Add EXPORT_SYMBOL_GPL(fsi_master_rescan) for =m builds. Thanks
kbuild test robot, I'll shout you your choice of coolant.
---
drivers/fsi/fsi-core.c | 10 --
drivers/fsi/fsi-master.h | 2 ++
2 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c
index a485864..c3d59d9 100644
--- a/drivers/fsi/fsi-core.c
+++ b/drivers/fsi/fsi-core.c
@@ -762,14 +762,20 @@ static void fsi_master_unscan(struct fsi_master *master)
device_for_each_child(>dev, NULL, fsi_master_remove_slave);
}
+int fsi_master_rescan(struct fsi_master *master)
+{
+ fsi_master_unscan(master);
+ return fsi_master_scan(master);
+}
+EXPORT_SYMBOL_GPL(fsi_master_rescan);
+
static ssize_t master_rescan_store(struct device *dev,
struct device_attribute *attr, const char *buf, size_t count)
{
struct fsi_master *master = to_fsi_master(dev);
int rc;
- fsi_master_unscan(master);
- rc = fsi_master_scan(master);
+ rc = fsi_master_rescan(master);
if (rc < 0)
return rc;
diff --git a/drivers/fsi/fsi-master.h b/drivers/fsi/fsi-master.h
index 12f7b11..18bd4ad 100644
--- a/drivers/fsi/fsi-master.h
+++ b/drivers/fsi/fsi-master.h
@@ -40,4 +40,6 @@ struct fsi_master {
extern int fsi_master_register(struct fsi_master *master);
extern void fsi_master_unregister(struct fsi_master *master);
+extern int fsi_master_rescan(struct fsi_master *master);
+
#endif /* DRIVERS_FSI_MASTER_H */
--
2.7.4
[PATCH v2 1/3] fsi: Add fsi_master_rescan()
We'll want non-core fsi code to trigger a rescan, so introduce a
non-static fsi_master_rescan() function. Use this for the existing
unscan/scan behaviour too.
Signed-off-by: Jeremy Kerr
Reviewed-by: Joel Stanley
Reviewed-by: Christopher Bostic
---
v2:
- Add EXPORT_SYMBOL_GPL(fsi_master_rescan) for =m builds. Thanks
kbuild test robot, I'll shout you your choice of coolant.
---
drivers/fsi/fsi-core.c | 10 --
drivers/fsi/fsi-master.h | 2 ++
2 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c
index a485864..c3d59d9 100644
--- a/drivers/fsi/fsi-core.c
+++ b/drivers/fsi/fsi-core.c
@@ -762,14 +762,20 @@ static void fsi_master_unscan(struct fsi_master *master)
device_for_each_child(>dev, NULL, fsi_master_remove_slave);
}
+int fsi_master_rescan(struct fsi_master *master)
+{
+ fsi_master_unscan(master);
+ return fsi_master_scan(master);
+}
+EXPORT_SYMBOL_GPL(fsi_master_rescan);
+
static ssize_t master_rescan_store(struct device *dev,
struct device_attribute *attr, const char *buf, size_t count)
{
struct fsi_master *master = to_fsi_master(dev);
int rc;
- fsi_master_unscan(master);
- rc = fsi_master_scan(master);
+ rc = fsi_master_rescan(master);
if (rc < 0)
return rc;
diff --git a/drivers/fsi/fsi-master.h b/drivers/fsi/fsi-master.h
index 12f7b11..18bd4ad 100644
--- a/drivers/fsi/fsi-master.h
+++ b/drivers/fsi/fsi-master.h
@@ -40,4 +40,6 @@ struct fsi_master {
extern int fsi_master_register(struct fsi_master *master);
extern void fsi_master_unregister(struct fsi_master *master);
+extern int fsi_master_rescan(struct fsi_master *master);
+
#endif /* DRIVERS_FSI_MASTER_H */
--
2.7.4
[PATCH 0/3] Add 'external mode' for GPIO-based FSI master
This series (on top of current char-misc-next) implements "external mode" (ie, support for FSI debug devices) for the GPIO-based FSI master driver. We implement this control in the GPIO master driver, as it has the mapping of raw GPIO pins to fsi control signals, and provides a mechanism for the kernel to retain exclusive access to those GPIOs. Cheers, Jeremy --- Jeremy Kerr (3): fsi: Add fsi_master_rescan() fsi/master-gpio: Add locking around gpio operations during break & link enable fsi/master-gpio: Add external mode .../ABI/testing/sysfs-driver-fsi-master-gpio | 10 +++ drivers/fsi/fsi-core.c | 9 ++- drivers/fsi/fsi-master-gpio.c | 85 +- drivers/fsi/fsi-master.h | 2 + 4 files changed, 102 insertions(+), 4 deletions(-) create mode 100644 Documentation/ABI/testing/sysfs-driver-fsi-master-gpio -- 2.7.4
[PATCH 1/3] fsi: Add fsi_master_rescan()
We'll want non-core fsi code to trigger a rescan, so introduce a
non-static fsi_master_rescan() function. Use this for the existing
unscan/scan behaviour too.
Signed-off-by: Jeremy Kerr <j...@ozlabs.org>
Reviewed-by: Joel Stanley <j...@jms.id.au>
Reviewed-by: Christopher Bostic <clbos...@linux.vnet.ibm.com>
---
drivers/fsi/fsi-core.c | 9 +++--
drivers/fsi/fsi-master.h | 2 ++
2 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c
index a485864..28d917e 100644
--- a/drivers/fsi/fsi-core.c
+++ b/drivers/fsi/fsi-core.c
@@ -762,14 +762,19 @@ static void fsi_master_unscan(struct fsi_master *master)
device_for_each_child(>dev, NULL, fsi_master_remove_slave);
}
+int fsi_master_rescan(struct fsi_master *master)
+{
+ fsi_master_unscan(master);
+ return fsi_master_scan(master);
+}
+
static ssize_t master_rescan_store(struct device *dev,
struct device_attribute *attr, const char *buf, size_t count)
{
struct fsi_master *master = to_fsi_master(dev);
int rc;
- fsi_master_unscan(master);
- rc = fsi_master_scan(master);
+ rc = fsi_master_rescan(master);
if (rc < 0)
return rc;
diff --git a/drivers/fsi/fsi-master.h b/drivers/fsi/fsi-master.h
index 12f7b11..18bd4ad 100644
--- a/drivers/fsi/fsi-master.h
+++ b/drivers/fsi/fsi-master.h
@@ -40,4 +40,6 @@ struct fsi_master {
extern int fsi_master_register(struct fsi_master *master);
extern void fsi_master_unregister(struct fsi_master *master);
+extern int fsi_master_rescan(struct fsi_master *master);
+
#endif /* DRIVERS_FSI_MASTER_H */
--
2.7.4
[PATCH 0/3] Add 'external mode' for GPIO-based FSI master
This series (on top of current char-misc-next) implements "external mode" (ie, support for FSI debug devices) for the GPIO-based FSI master driver. We implement this control in the GPIO master driver, as it has the mapping of raw GPIO pins to fsi control signals, and provides a mechanism for the kernel to retain exclusive access to those GPIOs. Cheers, Jeremy --- Jeremy Kerr (3): fsi: Add fsi_master_rescan() fsi/master-gpio: Add locking around gpio operations during break & link enable fsi/master-gpio: Add external mode .../ABI/testing/sysfs-driver-fsi-master-gpio | 10 +++ drivers/fsi/fsi-core.c | 9 ++- drivers/fsi/fsi-master-gpio.c | 85 +- drivers/fsi/fsi-master.h | 2 + 4 files changed, 102 insertions(+), 4 deletions(-) create mode 100644 Documentation/ABI/testing/sysfs-driver-fsi-master-gpio -- 2.7.4
[PATCH 1/3] fsi: Add fsi_master_rescan()
We'll want non-core fsi code to trigger a rescan, so introduce a
non-static fsi_master_rescan() function. Use this for the existing
unscan/scan behaviour too.
Signed-off-by: Jeremy Kerr
Reviewed-by: Joel Stanley
Reviewed-by: Christopher Bostic
---
drivers/fsi/fsi-core.c | 9 +++--
drivers/fsi/fsi-master.h | 2 ++
2 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c
index a485864..28d917e 100644
--- a/drivers/fsi/fsi-core.c
+++ b/drivers/fsi/fsi-core.c
@@ -762,14 +762,19 @@ static void fsi_master_unscan(struct fsi_master *master)
device_for_each_child(>dev, NULL, fsi_master_remove_slave);
}
+int fsi_master_rescan(struct fsi_master *master)
+{
+ fsi_master_unscan(master);
+ return fsi_master_scan(master);
+}
+
static ssize_t master_rescan_store(struct device *dev,
struct device_attribute *attr, const char *buf, size_t count)
{
struct fsi_master *master = to_fsi_master(dev);
int rc;
- fsi_master_unscan(master);
- rc = fsi_master_scan(master);
+ rc = fsi_master_rescan(master);
if (rc < 0)
return rc;
diff --git a/drivers/fsi/fsi-master.h b/drivers/fsi/fsi-master.h
index 12f7b11..18bd4ad 100644
--- a/drivers/fsi/fsi-master.h
+++ b/drivers/fsi/fsi-master.h
@@ -40,4 +40,6 @@ struct fsi_master {
extern int fsi_master_register(struct fsi_master *master);
extern void fsi_master_unregister(struct fsi_master *master);
+extern int fsi_master_rescan(struct fsi_master *master);
+
#endif /* DRIVERS_FSI_MASTER_H */
--
2.7.4
[PATCH 2/3] fsi/master-gpio: Add locking around gpio operations during break & link enable
Currently, we perform GPIO accesses in fsi_master_gpio_break and
fsi_master_link_enable, without holding cmd_lock. This change adds the
appropriate locking.
Signed-off-by: Jeremy Kerr <j...@ozlabs.org>
Reviewed-by: Joel Stanley <j...@jms.id.au>
Reviewed-by: Christopher Bostic <clbos...@linux.vnet.ibm.com>
---
drivers/fsi/fsi-master-gpio.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/drivers/fsi/fsi-master-gpio.c b/drivers/fsi/fsi-master-gpio.c
index ae26187..a6d602e 100644
--- a/drivers/fsi/fsi-master-gpio.c
+++ b/drivers/fsi/fsi-master-gpio.c
@@ -461,12 +461,14 @@ static int fsi_master_gpio_term(struct fsi_master
*_master,
static int fsi_master_gpio_break(struct fsi_master *_master, int link)
{
struct fsi_master_gpio *master = to_fsi_master_gpio(_master);
+ unsigned long flags;
if (link != 0)
return -ENODEV;
trace_fsi_master_gpio_break(master);
+ spin_lock_irqsave(>cmd_lock, flags);
set_sda_output(master, 1);
sda_out(master, 1);
clock_toggle(master, FSI_PRE_BREAK_CLOCKS);
@@ -475,6 +477,7 @@ static int fsi_master_gpio_break(struct fsi_master
*_master, int link)
echo_delay(master);
sda_out(master, 1);
clock_toggle(master, FSI_POST_BREAK_CLOCKS);
+ spin_unlock_irqrestore(>cmd_lock, flags);
/* Wait for logic reset to take effect */
udelay(200);
@@ -497,10 +500,14 @@ static void fsi_master_gpio_init(struct fsi_master_gpio
*master)
static int fsi_master_gpio_link_enable(struct fsi_master *_master, int link)
{
struct fsi_master_gpio *master = to_fsi_master_gpio(_master);
+ unsigned long flags;
if (link != 0)
return -ENODEV;
+
+ spin_lock_irqsave(>cmd_lock, flags);
gpiod_set_value(master->gpio_enable, 1);
+ spin_unlock_irqrestore(>cmd_lock, flags);
return 0;
}
--
2.7.4
[PATCH 3/3] fsi/master-gpio: Add external mode
This change introduces an 'external mode' for GPIO-based FSI masters,
allowing the clock and data lines to be driven by an external source.
For example, external mode is selected by a user when an external debug
device is attached to the FSI pins.
To do this, we need to set specific states for the trans, mux and enable
gpios, and prevent access to clk & data from the FSI core code (by
returning EBUSY).
External mode is controlled by a sysfs attribute, so add the relevent
information to Documentation/ABI/
Signed-off-by: Jeremy Kerr <j...@ozlabs.org>
Reviewed-by: Joel Stanley <j...@jms.id.au>
---
.../ABI/testing/sysfs-driver-fsi-master-gpio | 10 +++
drivers/fsi/fsi-master-gpio.c | 78 +-
2 files changed, 86 insertions(+), 2 deletions(-)
create mode 100644 Documentation/ABI/testing/sysfs-driver-fsi-master-gpio
diff --git a/Documentation/ABI/testing/sysfs-driver-fsi-master-gpio
b/Documentation/ABI/testing/sysfs-driver-fsi-master-gpio
new file mode 100644
index 000..9667bb4
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-driver-fsi-master-gpio
@@ -0,0 +1,10 @@
+What: /sys/bus/platform/devices/[..]/fsi-master-gpio/external_mode
+Date: June 2017
+KernelVersion: 4.12
+Contact:j...@ozlabs.org
+Description:
+Controls access arbitration for GPIO-based FSI master. A
+ value of 0 (the default) sets normal mode, where the
+ driver performs FSI bus transactions, 1 sets external mode,
+ where the FSI bus is driven externally (for example, by
+ a debug device).
diff --git a/drivers/fsi/fsi-master-gpio.c b/drivers/fsi/fsi-master-gpio.c
index a6d602e..88d26fe 100644
--- a/drivers/fsi/fsi-master-gpio.c
+++ b/drivers/fsi/fsi-master-gpio.c
@@ -59,6 +59,7 @@ struct fsi_master_gpio {
struct gpio_desc*gpio_trans;/* Voltage translator */
struct gpio_desc*gpio_enable; /* FSI enable */
struct gpio_desc*gpio_mux; /* Mux control */
+ boolexternal_mode;
};
#define CREATE_TRACE_POINTS
@@ -411,6 +412,12 @@ static int fsi_master_gpio_xfer(struct fsi_master_gpio
*master, uint8_t slave,
int rc;
spin_lock_irqsave(>cmd_lock, flags);
+
+ if (master->external_mode) {
+ spin_unlock_irqrestore(>cmd_lock, flags);
+ return -EBUSY;
+ }
+
serial_out(master, cmd);
echo_delay(master);
rc = poll_for_response(master, slave, resp_len, resp);
@@ -469,6 +476,10 @@ static int fsi_master_gpio_break(struct fsi_master
*_master, int link)
trace_fsi_master_gpio_break(master);
spin_lock_irqsave(>cmd_lock, flags);
+ if (master->external_mode) {
+ spin_unlock_irqrestore(>cmd_lock, flags);
+ return -EBUSY;
+ }
set_sda_output(master, 1);
sda_out(master, 1);
clock_toggle(master, FSI_PRE_BREAK_CLOCKS);
@@ -497,25 +508,84 @@ static void fsi_master_gpio_init(struct fsi_master_gpio
*master)
clock_zeros(master, FSI_INIT_CLOCKS);
}
+static void fsi_master_gpio_init_external(struct fsi_master_gpio *master)
+{
+ gpiod_direction_output(master->gpio_mux, 0);
+ gpiod_direction_output(master->gpio_trans, 0);
+ gpiod_direction_output(master->gpio_enable, 1);
+ gpiod_direction_input(master->gpio_clk);
+ gpiod_direction_input(master->gpio_data);
+}
+
static int fsi_master_gpio_link_enable(struct fsi_master *_master, int link)
{
struct fsi_master_gpio *master = to_fsi_master_gpio(_master);
unsigned long flags;
+ int rc = -EBUSY;
if (link != 0)
return -ENODEV;
spin_lock_irqsave(>cmd_lock, flags);
- gpiod_set_value(master->gpio_enable, 1);
+ if (!master->external_mode) {
+ gpiod_set_value(master->gpio_enable, 1);
+ rc = 0;
+ }
spin_unlock_irqrestore(>cmd_lock, flags);
- return 0;
+ return rc;
+}
+
+static ssize_t external_mode_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct fsi_master_gpio *master = dev_get_drvdata(dev);
+
+ return snprintf(buf, PAGE_SIZE - 1, "%u",
+ master->external_mode ? 1 : 0);
+}
+
+static ssize_t external_mode_store(struct device *dev,
+ struct device_attribute *attr, const char *buf, size_t count)
+{
+ struct fsi_master_gpio *master = dev_get_drvdata(dev);
+ unsigned long flags, val;
+ bool external_mode;
+ int err;
+
+ err = kstrtoul(buf, 0, );
+ if (err)
+ return err;
+
+ external_mode = !!val;
+
+ spin_lock_irqsave(>cmd_lock, flags);
+
+ if (external_mode == master->external_mode) {
+ spin_unlock_irqrestore(>cmd
[PATCH 2/3] fsi/master-gpio: Add locking around gpio operations during break & link enable
Currently, we perform GPIO accesses in fsi_master_gpio_break and
fsi_master_link_enable, without holding cmd_lock. This change adds the
appropriate locking.
Signed-off-by: Jeremy Kerr
Reviewed-by: Joel Stanley
Reviewed-by: Christopher Bostic
---
drivers/fsi/fsi-master-gpio.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/drivers/fsi/fsi-master-gpio.c b/drivers/fsi/fsi-master-gpio.c
index ae26187..a6d602e 100644
--- a/drivers/fsi/fsi-master-gpio.c
+++ b/drivers/fsi/fsi-master-gpio.c
@@ -461,12 +461,14 @@ static int fsi_master_gpio_term(struct fsi_master
*_master,
static int fsi_master_gpio_break(struct fsi_master *_master, int link)
{
struct fsi_master_gpio *master = to_fsi_master_gpio(_master);
+ unsigned long flags;
if (link != 0)
return -ENODEV;
trace_fsi_master_gpio_break(master);
+ spin_lock_irqsave(>cmd_lock, flags);
set_sda_output(master, 1);
sda_out(master, 1);
clock_toggle(master, FSI_PRE_BREAK_CLOCKS);
@@ -475,6 +477,7 @@ static int fsi_master_gpio_break(struct fsi_master
*_master, int link)
echo_delay(master);
sda_out(master, 1);
clock_toggle(master, FSI_POST_BREAK_CLOCKS);
+ spin_unlock_irqrestore(>cmd_lock, flags);
/* Wait for logic reset to take effect */
udelay(200);
@@ -497,10 +500,14 @@ static void fsi_master_gpio_init(struct fsi_master_gpio
*master)
static int fsi_master_gpio_link_enable(struct fsi_master *_master, int link)
{
struct fsi_master_gpio *master = to_fsi_master_gpio(_master);
+ unsigned long flags;
if (link != 0)
return -ENODEV;
+
+ spin_lock_irqsave(>cmd_lock, flags);
gpiod_set_value(master->gpio_enable, 1);
+ spin_unlock_irqrestore(>cmd_lock, flags);
return 0;
}
--
2.7.4
[PATCH 3/3] fsi/master-gpio: Add external mode
This change introduces an 'external mode' for GPIO-based FSI masters,
allowing the clock and data lines to be driven by an external source.
For example, external mode is selected by a user when an external debug
device is attached to the FSI pins.
To do this, we need to set specific states for the trans, mux and enable
gpios, and prevent access to clk & data from the FSI core code (by
returning EBUSY).
External mode is controlled by a sysfs attribute, so add the relevent
information to Documentation/ABI/
Signed-off-by: Jeremy Kerr
Reviewed-by: Joel Stanley
---
.../ABI/testing/sysfs-driver-fsi-master-gpio | 10 +++
drivers/fsi/fsi-master-gpio.c | 78 +-
2 files changed, 86 insertions(+), 2 deletions(-)
create mode 100644 Documentation/ABI/testing/sysfs-driver-fsi-master-gpio
diff --git a/Documentation/ABI/testing/sysfs-driver-fsi-master-gpio
b/Documentation/ABI/testing/sysfs-driver-fsi-master-gpio
new file mode 100644
index 000..9667bb4
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-driver-fsi-master-gpio
@@ -0,0 +1,10 @@
+What: /sys/bus/platform/devices/[..]/fsi-master-gpio/external_mode
+Date: June 2017
+KernelVersion: 4.12
+Contact:j...@ozlabs.org
+Description:
+Controls access arbitration for GPIO-based FSI master. A
+ value of 0 (the default) sets normal mode, where the
+ driver performs FSI bus transactions, 1 sets external mode,
+ where the FSI bus is driven externally (for example, by
+ a debug device).
diff --git a/drivers/fsi/fsi-master-gpio.c b/drivers/fsi/fsi-master-gpio.c
index a6d602e..88d26fe 100644
--- a/drivers/fsi/fsi-master-gpio.c
+++ b/drivers/fsi/fsi-master-gpio.c
@@ -59,6 +59,7 @@ struct fsi_master_gpio {
struct gpio_desc*gpio_trans;/* Voltage translator */
struct gpio_desc*gpio_enable; /* FSI enable */
struct gpio_desc*gpio_mux; /* Mux control */
+ boolexternal_mode;
};
#define CREATE_TRACE_POINTS
@@ -411,6 +412,12 @@ static int fsi_master_gpio_xfer(struct fsi_master_gpio
*master, uint8_t slave,
int rc;
spin_lock_irqsave(>cmd_lock, flags);
+
+ if (master->external_mode) {
+ spin_unlock_irqrestore(>cmd_lock, flags);
+ return -EBUSY;
+ }
+
serial_out(master, cmd);
echo_delay(master);
rc = poll_for_response(master, slave, resp_len, resp);
@@ -469,6 +476,10 @@ static int fsi_master_gpio_break(struct fsi_master
*_master, int link)
trace_fsi_master_gpio_break(master);
spin_lock_irqsave(>cmd_lock, flags);
+ if (master->external_mode) {
+ spin_unlock_irqrestore(>cmd_lock, flags);
+ return -EBUSY;
+ }
set_sda_output(master, 1);
sda_out(master, 1);
clock_toggle(master, FSI_PRE_BREAK_CLOCKS);
@@ -497,25 +508,84 @@ static void fsi_master_gpio_init(struct fsi_master_gpio
*master)
clock_zeros(master, FSI_INIT_CLOCKS);
}
+static void fsi_master_gpio_init_external(struct fsi_master_gpio *master)
+{
+ gpiod_direction_output(master->gpio_mux, 0);
+ gpiod_direction_output(master->gpio_trans, 0);
+ gpiod_direction_output(master->gpio_enable, 1);
+ gpiod_direction_input(master->gpio_clk);
+ gpiod_direction_input(master->gpio_data);
+}
+
static int fsi_master_gpio_link_enable(struct fsi_master *_master, int link)
{
struct fsi_master_gpio *master = to_fsi_master_gpio(_master);
unsigned long flags;
+ int rc = -EBUSY;
if (link != 0)
return -ENODEV;
spin_lock_irqsave(>cmd_lock, flags);
- gpiod_set_value(master->gpio_enable, 1);
+ if (!master->external_mode) {
+ gpiod_set_value(master->gpio_enable, 1);
+ rc = 0;
+ }
spin_unlock_irqrestore(>cmd_lock, flags);
- return 0;
+ return rc;
+}
+
+static ssize_t external_mode_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct fsi_master_gpio *master = dev_get_drvdata(dev);
+
+ return snprintf(buf, PAGE_SIZE - 1, "%u",
+ master->external_mode ? 1 : 0);
+}
+
+static ssize_t external_mode_store(struct device *dev,
+ struct device_attribute *attr, const char *buf, size_t count)
+{
+ struct fsi_master_gpio *master = dev_get_drvdata(dev);
+ unsigned long flags, val;
+ bool external_mode;
+ int err;
+
+ err = kstrtoul(buf, 0, );
+ if (err)
+ return err;
+
+ external_mode = !!val;
+
+ spin_lock_irqsave(>cmd_lock, flags);
+
+ if (external_mode == master->external_mode) {
+ spin_unlock_irqrestore(>cmd_lock, flags);
+ return count;
+
Re: [PATCH v8 16/24] drivers/fsi: Add tracepoints for low-level operations
Hi Steven, Thanks for checking this out. >> +TRACE_EVENT(fsi_master_write, >> +TP_PROTO(const struct fsi_master *master, int link, int id, >> +uint32_t addr, size_t size, const void *data), >> +TP_ARGS(master, link, id, addr, size, data), >> +TP_STRUCT__entry( >> +__field(int,master_idx) >> +__field(int,link) >> +__field(int,id) >> +__field(__u32, addr) >> +__field(size_t, size) >> +__field(__u32, data) >> +), >> +TP_fast_assign( >> +__entry->master_idx = master->idx; >> +__entry->link = link; >> +__entry->id = id; >> +__entry->addr = addr; >> +__entry->size = size; >> +__entry->data = 0; >> +memcpy(&__entry->data, data, size); > > > Um, can size ever be greater than 4? If so, this is a bug. No, size will only ever be 1, 2, or 4, as they're the only valid FSI bus transactions. Hence storing ->data as a u32 to keep things simple. > I think you may want to use a dynamic array here. > > TP_STRUCT__entry( > [..] > __dynamic_array(char, data, size) > [..] > > TP_fast_assign( > [..] > memcpy(__get_dynamic_array(data), data, size); > [..] > > TP_printk(... > [..] > __entry->size, > __get_dynamic_array(data) > > You may also want to look at __print_array() too. The %ph specifier seems to do a decent job though... Cheers, Jeremy
Re: [PATCH v8 16/24] drivers/fsi: Add tracepoints for low-level operations
Hi Steven, Thanks for checking this out. >> +TRACE_EVENT(fsi_master_write, >> +TP_PROTO(const struct fsi_master *master, int link, int id, >> +uint32_t addr, size_t size, const void *data), >> +TP_ARGS(master, link, id, addr, size, data), >> +TP_STRUCT__entry( >> +__field(int,master_idx) >> +__field(int,link) >> +__field(int,id) >> +__field(__u32, addr) >> +__field(size_t, size) >> +__field(__u32, data) >> +), >> +TP_fast_assign( >> +__entry->master_idx = master->idx; >> +__entry->link = link; >> +__entry->id = id; >> +__entry->addr = addr; >> +__entry->size = size; >> +__entry->data = 0; >> +memcpy(&__entry->data, data, size); > > > Um, can size ever be greater than 4? If so, this is a bug. No, size will only ever be 1, 2, or 4, as they're the only valid FSI bus transactions. Hence storing ->data as a u32 to keep things simple. > I think you may want to use a dynamic array here. > > TP_STRUCT__entry( > [..] > __dynamic_array(char, data, size) > [..] > > TP_fast_assign( > [..] > memcpy(__get_dynamic_array(data), data, size); > [..] > > TP_printk(... > [..] > __entry->size, > __get_dynamic_array(data) > > You may also want to look at __print_array() too. The %ph specifier seems to do a decent job though... Cheers, Jeremy
Re: [PATCH v6 19/23] drivers/fsi: Add GPIO based FSI master
Hi Chris, > I don't think we'd want this per master. The lock is for the 'top' > master issuing commands. Only the top master can initiate any > transactions on the bus to any devices connected downstream. Downstream > masters such as hub masters, etc... cannot initiate a command. I think what Joel meant there was that we have it per *GPIO* master; if there are two GPIO masters on a system, there's no need to provide mutual exclusion to each (separate) set of GPIOs. To implement this, we'd just move the lock into struct fsi_master_gpio. Cheers, Jeremy
Re: [PATCH v6 19/23] drivers/fsi: Add GPIO based FSI master
Hi Chris, > I don't think we'd want this per master. The lock is for the 'top' > master issuing commands. Only the top master can initiate any > transactions on the bus to any devices connected downstream. Downstream > masters such as hub masters, etc... cannot initiate a command. I think what Joel meant there was that we have it per *GPIO* master; if there are two GPIO masters on a system, there's no need to provide mutual exclusion to each (separate) set of GPIOs. To implement this, we'd just move the lock into struct fsi_master_gpio. Cheers, Jeremy
Re: [PATCH v3 01/18] drivers/fsi: Add empty fsi bus definitions
Hi Geert, >> --- /dev/null >> +++ b/drivers/fsi/Kconfig >> @@ -0,0 +1,12 @@ >> +# >> +# FSI subsystem >> +# >> + >> +menu "FSI support" >> + >> +config FSI >> + tristate "FSI support" > > I guess this should depend on some POWER symbol || COMPILE_TEST? No, this is pretty hardware-independent - all it requires on the HW side is two GPIOs. Although the FSI slave devices are typically only present on POWER systems, they can be driven from anything, with currently implementations being on ARM and 32-bit powerpc. Cheers, Jeremy
Re: [PATCH v3 01/18] drivers/fsi: Add empty fsi bus definitions
Hi Geert, >> --- /dev/null >> +++ b/drivers/fsi/Kconfig >> @@ -0,0 +1,12 @@ >> +# >> +# FSI subsystem >> +# >> + >> +menu "FSI support" >> + >> +config FSI >> + tristate "FSI support" > > I guess this should depend on some POWER symbol || COMPILE_TEST? No, this is pretty hardware-independent - all it requires on the HW side is two GPIOs. Although the FSI slave devices are typically only present on POWER systems, they can be driven from anything, with currently implementations being on ARM and 32-bit powerpc. Cheers, Jeremy
[tip:irq/core] genirq: Clarify logic calculating bogus irqreturn_t values
Commit-ID: 5d4bac9a5f4ef24b2482529bda6661a58e5b5b65
Gitweb: http://git.kernel.org/tip/5d4bac9a5f4ef24b2482529bda6661a58e5b5b65
Author: Jeremy Kerr <j...@ozlabs.org>
AuthorDate: Thu, 16 Feb 2017 12:24:09 +0800
Committer: Thomas Gleixner <t...@linutronix.de>
CommitDate: Thu, 16 Feb 2017 15:32:19 +0100
genirq: Clarify logic calculating bogus irqreturn_t values
Although irqreturn_t is an enum, we treat it (and its enumeration
constants) as a bitmask.
However, bad_action_ret() uses a less-than operator to determine whether
an irqreturn_t falls within allowable bit values, which means we need to
know the signededness of an enum type to read the logic, which is
implementation-dependent.
This change explicitly uses an unsigned type for the comparison. We do
this instead of changing to a bitwise test, as the latter compiles to
increased instructions in this hot path.
It looks like we get the correct behaviour currently (bad_action_ret(-1)
returns 1), so this is purely a readability fix.
Signed-off-by: Jeremy Kerr <j...@ozlabs.org>
Link: http://lkml.kernel.org/r/1487219049-4061-1-git-send-email...@ozlabs.org
Signed-off-by: Thomas Gleixner <t...@linutronix.de>
---
kernel/irq/spurious.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/kernel/irq/spurious.c b/kernel/irq/spurious.c
index 5707f97..061ba7e 100644
--- a/kernel/irq/spurious.c
+++ b/kernel/irq/spurious.c
@@ -175,7 +175,9 @@ out:
static inline int bad_action_ret(irqreturn_t action_ret)
{
- if (likely(action_ret <= (IRQ_HANDLED | IRQ_WAKE_THREAD)))
+ unsigned int r = action_ret;
+
+ if (likely(r <= (IRQ_HANDLED | IRQ_WAKE_THREAD)))
return 0;
return 1;
}
[tip:irq/core] genirq: Clarify logic calculating bogus irqreturn_t values
Commit-ID: 5d4bac9a5f4ef24b2482529bda6661a58e5b5b65
Gitweb: http://git.kernel.org/tip/5d4bac9a5f4ef24b2482529bda6661a58e5b5b65
Author: Jeremy Kerr
AuthorDate: Thu, 16 Feb 2017 12:24:09 +0800
Committer: Thomas Gleixner
CommitDate: Thu, 16 Feb 2017 15:32:19 +0100
genirq: Clarify logic calculating bogus irqreturn_t values
Although irqreturn_t is an enum, we treat it (and its enumeration
constants) as a bitmask.
However, bad_action_ret() uses a less-than operator to determine whether
an irqreturn_t falls within allowable bit values, which means we need to
know the signededness of an enum type to read the logic, which is
implementation-dependent.
This change explicitly uses an unsigned type for the comparison. We do
this instead of changing to a bitwise test, as the latter compiles to
increased instructions in this hot path.
It looks like we get the correct behaviour currently (bad_action_ret(-1)
returns 1), so this is purely a readability fix.
Signed-off-by: Jeremy Kerr
Link: http://lkml.kernel.org/r/1487219049-4061-1-git-send-email...@ozlabs.org
Signed-off-by: Thomas Gleixner
---
kernel/irq/spurious.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/kernel/irq/spurious.c b/kernel/irq/spurious.c
index 5707f97..061ba7e 100644
--- a/kernel/irq/spurious.c
+++ b/kernel/irq/spurious.c
@@ -175,7 +175,9 @@ out:
static inline int bad_action_ret(irqreturn_t action_ret)
{
- if (likely(action_ret <= (IRQ_HANDLED | IRQ_WAKE_THREAD)))
+ unsigned int r = action_ret;
+
+ if (likely(r <= (IRQ_HANDLED | IRQ_WAKE_THREAD)))
return 0;
return 1;
}
[PATCH] irq: clarify logic calculating bogus irqreturn_t values
Although irqreturn_t is an enum, we treat it (and its enumeration
constants) as a bitmask.
However, bad_action_ret() uses a less-than operator to determine whether
an irqreturn_t falls within allowable bit values, which means we need to
know the signededness of an enum type to read the logic, which is
implementation-dependent.
This change explicitly uses an unsigned type for the comparison. We do
this instead of changing to a bitwise test, as the latter compiles to
increased instructions in this hot path.
It looks like we get the correct behaviour currently (bad_action_ret(-1)
returns 1), so this is purely a readability fix.
Signed-off-by: Jeremy Kerr <j...@ozlabs.org>
---
kernel/irq/spurious.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/kernel/irq/spurious.c b/kernel/irq/spurious.c
index 5707f97..061ba7e 100644
--- a/kernel/irq/spurious.c
+++ b/kernel/irq/spurious.c
@@ -175,7 +175,9 @@ static void poll_spurious_irqs(unsigned long dummy)
static inline int bad_action_ret(irqreturn_t action_ret)
{
- if (likely(action_ret <= (IRQ_HANDLED | IRQ_WAKE_THREAD)))
+ unsigned int r = action_ret;
+
+ if (likely(r <= (IRQ_HANDLED | IRQ_WAKE_THREAD)))
return 0;
return 1;
}
--
2.7.4
[PATCH] irq: clarify logic calculating bogus irqreturn_t values
Although irqreturn_t is an enum, we treat it (and its enumeration
constants) as a bitmask.
However, bad_action_ret() uses a less-than operator to determine whether
an irqreturn_t falls within allowable bit values, which means we need to
know the signededness of an enum type to read the logic, which is
implementation-dependent.
This change explicitly uses an unsigned type for the comparison. We do
this instead of changing to a bitwise test, as the latter compiles to
increased instructions in this hot path.
It looks like we get the correct behaviour currently (bad_action_ret(-1)
returns 1), so this is purely a readability fix.
Signed-off-by: Jeremy Kerr
---
kernel/irq/spurious.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/kernel/irq/spurious.c b/kernel/irq/spurious.c
index 5707f97..061ba7e 100644
--- a/kernel/irq/spurious.c
+++ b/kernel/irq/spurious.c
@@ -175,7 +175,9 @@ static void poll_spurious_irqs(unsigned long dummy)
static inline int bad_action_ret(irqreturn_t action_ret)
{
- if (likely(action_ret <= (IRQ_HANDLED | IRQ_WAKE_THREAD)))
+ unsigned int r = action_ret;
+
+ if (likely(r <= (IRQ_HANDLED | IRQ_WAKE_THREAD)))
return 0;
return 1;
}
--
2.7.4
Re: [PATCH v2 15/18] drivers/fsi: Add documentation for GPIO based FSI master
Hi Chris,
>From this:
>> +
>> +The standard FSI master node
>> +
>> +This node describes a FSI master implmemented fully in hardware
>> +with dedicated input/output pins required for its function (i.e.
>> +not using generic GPIO pins).
>> +Required property:
>> +compatible = "ibm,fsi-master"
and this:
>> +Example:
>> +
>> +fsi-master {
>> +compatible = "ibm,fsi-master-gpio", "ibm,fsi-master";
>
> From the description, these should be mutually exclusive.
I agree with Rob here. The intention is for "ibm,fsi-master" to be an
abstract master -- simply indicating that this node describes a master,
with no specific implementation, and "ibm,fsi-master-gpio" to be a
GPIO-based implementation. A hardware-based FSI master would have a
different compatible value, based on the hardware.
We should remove references to implementations in the "The standard FSI
master node" section, because this is independent of implementation.
>> +clk-gpios = < 0>, < 6>;
>> +data-gpios = < 1>, < 7>;
>> +enable-gpios = < 2>, < 8>;
>> +trans-gpios = < 3>, < 9>;
>> +mux-gpios = < 4>, < 10>;
Do we support multiple-link masters? This example implies a 2-link
master.
Cheers,
Jeremy
Re: [PATCH v2 15/18] drivers/fsi: Add documentation for GPIO based FSI master
Hi Chris,
>From this:
>> +
>> +The standard FSI master node
>> +
>> +This node describes a FSI master implmemented fully in hardware
>> +with dedicated input/output pins required for its function (i.e.
>> +not using generic GPIO pins).
>> +Required property:
>> +compatible = "ibm,fsi-master"
and this:
>> +Example:
>> +
>> +fsi-master {
>> +compatible = "ibm,fsi-master-gpio", "ibm,fsi-master";
>
> From the description, these should be mutually exclusive.
I agree with Rob here. The intention is for "ibm,fsi-master" to be an
abstract master -- simply indicating that this node describes a master,
with no specific implementation, and "ibm,fsi-master-gpio" to be a
GPIO-based implementation. A hardware-based FSI master would have a
different compatible value, based on the hardware.
We should remove references to implementations in the "The standard FSI
master node" section, because this is independent of implementation.
>> +clk-gpios = < 0>, < 6>;
>> +data-gpios = < 1>, < 7>;
>> +enable-gpios = < 2>, < 8>;
>> +trans-gpios = < 3>, < 9>;
>> +mux-gpios = < 4>, < 10>;
Do we support multiple-link masters? This example implies a 2-link
master.
Cheers,
Jeremy
Re: [PATCH 16/16] drivers/fsi: Add GPIO based FSI master
Hi Chris,
> +static ssize_t store_scan(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf,
> + size_t count)
> +{
> + struct fsi_master_gpio *master = dev_get_drvdata(dev);
> +
> + fsi_master_gpio_init(master);
> +
> + /* clear out any old scan data if present */
> + fsi_master_unregister(>master);
> + fsi_master_register(>master);
> +
> + return count;
> +}
> +
> +static DEVICE_ATTR(scan, 0200, NULL, store_scan);
I think it would make more sense to have the scan attribute populated by
the fsi core; we want this on all masters, not just GPIO.
Currently, the only GPIO-master-specific functionality here is the
fsi_master_gpio_init() - but isn't this something that we can do at
probe time instead?
> +static int fsi_master_gpio_probe(struct platform_device *pdev)
> +{
> + struct fsi_master_gpio *master;
> + struct gpio_desc *gpio;
> +
> + master = devm_kzalloc(>dev, sizeof(*master), GFP_KERNEL);
> + if (!master)
> + return -ENOMEM;
We should be populating master->dev.parent, see
https://github.com/jk-ozlabs/linux/commit/5225d6c47
> + /* Optional pins */
> +
> + gpio = devm_gpiod_get(>dev, "trans", 0);
> + if (IS_ERR(gpio))
> + dev_dbg(>dev, "probe: failed to get trans pin\n");
> + else
> + master->gpio_trans = gpio;
I found devm_gpiod_get_optional(), which might make this a little
neater.
Cheers,
Jeremy
Re: [PATCH 16/16] drivers/fsi: Add GPIO based FSI master
Hi Chris,
> +static ssize_t store_scan(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf,
> + size_t count)
> +{
> + struct fsi_master_gpio *master = dev_get_drvdata(dev);
> +
> + fsi_master_gpio_init(master);
> +
> + /* clear out any old scan data if present */
> + fsi_master_unregister(>master);
> + fsi_master_register(>master);
> +
> + return count;
> +}
> +
> +static DEVICE_ATTR(scan, 0200, NULL, store_scan);
I think it would make more sense to have the scan attribute populated by
the fsi core; we want this on all masters, not just GPIO.
Currently, the only GPIO-master-specific functionality here is the
fsi_master_gpio_init() - but isn't this something that we can do at
probe time instead?
> +static int fsi_master_gpio_probe(struct platform_device *pdev)
> +{
> + struct fsi_master_gpio *master;
> + struct gpio_desc *gpio;
> +
> + master = devm_kzalloc(>dev, sizeof(*master), GFP_KERNEL);
> + if (!master)
> + return -ENOMEM;
We should be populating master->dev.parent, see
https://github.com/jk-ozlabs/linux/commit/5225d6c47
> + /* Optional pins */
> +
> + gpio = devm_gpiod_get(>dev, "trans", 0);
> + if (IS_ERR(gpio))
> + dev_dbg(>dev, "probe: failed to get trans pin\n");
> + else
> + master->gpio_trans = gpio;
I found devm_gpiod_get_optional(), which might make this a little
neater.
Cheers,
Jeremy
Re: [PATCH 08/16] drivers/fsi: Add crc4 helpers
Hi Greg, > Why not just create lib/crc4.c with these functions, like the other crc > functions in the kernel? Two (bad) reasons: - The crc4 implementation here is pretty specific to the FSI usage (only supporting 4-bit-sized chunks), to keep the math & lookup table simple - I'm lazy So yes, we should spend the effort now to make this generic enough for a lib/crc4.c. Would we want to support different values for the polynomial? Chris: do you want me to to that, or will you? Cheers, Jeremy
Re: [PATCH 08/16] drivers/fsi: Add crc4 helpers
Hi Greg, > Why not just create lib/crc4.c with these functions, like the other crc > functions in the kernel? Two (bad) reasons: - The crc4 implementation here is pretty specific to the FSI usage (only supporting 4-bit-sized chunks), to keep the math & lookup table simple - I'm lazy So yes, we should spend the effort now to make this generic enough for a lib/crc4.c. Would we want to support different values for the polynomial? Chris: do you want me to to that, or will you? Cheers, Jeremy
Re: [PATCH 05/16] drivers/fsi: Add fake master driver
Hi Mark & Chris,
> On Tue, Dec 06, 2016 at 06:14:26PM -0600, Chris Bostic wrote:
>> From: Jeremy Kerr <j...@ozlabs.org>
>>
>> For debugging, add a fake master driver, that only supports reads,
>> returning a fixed set of data.
>
>> +config FSI_MASTER_FAKE
>> +tristate "Fake FSI master"
>> +depends on FSI
>> +---help---
>> +This option enables a fake FSI master driver for debugging.
>> +endif
>
>> +static const struct of_device_id fsi_master_fake_match[] = {
>> +{ .compatible = "ibm,fsi-master-fake" },
>> +{ },
>> +};
>
> NAK.
>
> DT should be treated as an ABI, and should describe the HW explicitly.
> This makes no sense. This is also missing a binding document.
>
> Have your module take a module parameter allowing you to bind it to
> arbitrary devices, or do something like what PCI does where you can
> bind/unbind arbitrary drivers to devices using sysfs.
This driver is purely for testing the FSI engine scan code; we could
probably just drop this patch since I suspect that it's no longer useful
(now that we have an actual master driver).
If we do want to keep it though, I'd say we remove the device tree
dependency; all this is doing at the moment is triggering the ->probe,
and there are better ways to do that.
Cheers,
Jeremy
Re: [PATCH 05/16] drivers/fsi: Add fake master driver
Hi Mark & Chris,
> On Tue, Dec 06, 2016 at 06:14:26PM -0600, Chris Bostic wrote:
>> From: Jeremy Kerr
>>
>> For debugging, add a fake master driver, that only supports reads,
>> returning a fixed set of data.
>
>> +config FSI_MASTER_FAKE
>> +tristate "Fake FSI master"
>> +depends on FSI
>> +---help---
>> +This option enables a fake FSI master driver for debugging.
>> +endif
>
>> +static const struct of_device_id fsi_master_fake_match[] = {
>> +{ .compatible = "ibm,fsi-master-fake" },
>> +{ },
>> +};
>
> NAK.
>
> DT should be treated as an ABI, and should describe the HW explicitly.
> This makes no sense. This is also missing a binding document.
>
> Have your module take a module parameter allowing you to bind it to
> arbitrary devices, or do something like what PCI does where you can
> bind/unbind arbitrary drivers to devices using sysfs.
This driver is purely for testing the FSI engine scan code; we could
probably just drop this patch since I suspect that it's no longer useful
(now that we have an actual master driver).
If we do want to keep it though, I'd say we remove the device tree
dependency; all this is doing at the moment is triggering the ->probe,
and there are better ways to do that.
Cheers,
Jeremy
Re: [RFC 0/3] extend kexec_file_load system call
Hi Thiago, > So even if not ideal, the solution above is desirable for powerpc. We would > like to preserve the ability of allowing userspace to pass parameters to the > OS via the DTB, even if secure boot is enabled. > > I would like to turn the above into a proposal: > > Extend the syscall as shown in this RFC from Takahiro AKASHI, but instead of > accepting a complete DTB from userspace, the syscall accepts a DTB > containing only a /chosen node. If the DTB contains any other node, the > syscall fails with EINVAL. If the DTB contains any subnode in /chosen, or if > there's a compatible or device_type property in /chosen, the syscall fails > with EINVAL as well. This works for me. We could even have it as just a DTB fragment that is merged *at* the /chosen/ node of the kernel-device tree - so would not contain a /chosen node itself, and it would be impossible to provide nodes outside of /chosen. Either is fine. Thanks! Jeremy
Re: [RFC 0/3] extend kexec_file_load system call
Hi Thiago, > So even if not ideal, the solution above is desirable for powerpc. We would > like to preserve the ability of allowing userspace to pass parameters to the > OS via the DTB, even if secure boot is enabled. > > I would like to turn the above into a proposal: > > Extend the syscall as shown in this RFC from Takahiro AKASHI, but instead of > accepting a complete DTB from userspace, the syscall accepts a DTB > containing only a /chosen node. If the DTB contains any other node, the > syscall fails with EINVAL. If the DTB contains any subnode in /chosen, or if > there's a compatible or device_type property in /chosen, the syscall fails > with EINVAL as well. This works for me. We could even have it as just a DTB fragment that is merged *at* the /chosen/ node of the kernel-device tree - so would not contain a /chosen node itself, and it would be impossible to provide nodes outside of /chosen. Either is fine. Thanks! Jeremy
Re: [PATCH] fbcon: warn on invalid cursor blink intervals
Hi Ming, >Then looks there are two fix patches acked & tested: > > - the patch in this thread > - another one "[PATCH] tty: vt: Fix soft lockup in fbcon cursor >blink timer." > https://lkml.org/lkml/2016/5/17/455 > >So which one will be pushed to linus? Not that it's my call, but we may want both; the first as a safety measure to prevent an invalid cur_blink_jiffies ever being set, and the second one to actually fix the initialisation of vc_cur_blink_ms (and address the warning introduced by the first). I guess we could just go with the latter for stable... Cheers, Jeremy
Re: [PATCH] fbcon: warn on invalid cursor blink intervals
Hi Ming, >Then looks there are two fix patches acked & tested: > > - the patch in this thread > - another one "[PATCH] tty: vt: Fix soft lockup in fbcon cursor >blink timer." > https://lkml.org/lkml/2016/5/17/455 > >So which one will be pushed to linus? Not that it's my call, but we may want both; the first as a safety measure to prevent an invalid cur_blink_jiffies ever being set, and the second one to actually fix the initialisation of vc_cur_blink_ms (and address the warning introduced by the first). I guess we could just go with the latter for stable... Cheers, Jeremy
Re: [PATCH] fbcon: warn on invalid cursor blink intervals
Hi Ming, > Not sure this one is needed for stable because it justs dumps > a warning, and not set a valid period to ops->cur_blink_jiffies. > > So I guess other fix patch is still required for the soft lockup > issue, right? The main thing is that we don't set cur_blink_jiffies to the < 50ms value. As far as I can tell, it means we'll still get the original default, set in fbcon_startup(): ops->cur_blink_jiffies = HZ / 5; And so don't end up spinning on the timer expiry. Cheers, Jeremy
Re: [PATCH] fbcon: warn on invalid cursor blink intervals
Hi Ming, > Not sure this one is needed for stable because it justs dumps > a warning, and not set a valid period to ops->cur_blink_jiffies. > > So I guess other fix patch is still required for the soft lockup > issue, right? The main thing is that we don't set cur_blink_jiffies to the < 50ms value. As far as I can tell, it means we'll still get the original default, set in fbcon_startup(): ops->cur_blink_jiffies = HZ / 5; And so don't end up spinning on the timer expiry. Cheers, Jeremy
Re: [PATCH] fbcon: warn on invalid cursor blink intervals
Hi Scot, > Two systems are locking on boot [1] because ops->cur_blink_jiffies > is set to zero from vc->vc_cur_blink_ms. > > Ignore such invalid intervals and log a warning. This prevents a lockup on AST BMC machines, but (as expected) generates a warning against the fbcon driver, which is a significantly better result. Tested-by: Jeremy Kerr <j...@ozlabs.org> [now to sort out the issue in the ast driver...] Cheers, Jeremy
Re: [PATCH] fbcon: warn on invalid cursor blink intervals
Hi Scot, > Two systems are locking on boot [1] because ops->cur_blink_jiffies > is set to zero from vc->vc_cur_blink_ms. > > Ignore such invalid intervals and log a warning. This prevents a lockup on AST BMC machines, but (as expected) generates a warning against the fbcon driver, which is a significantly better result. Tested-by: Jeremy Kerr [now to sort out the issue in the ast driver...] Cheers, Jeremy
Re: [PATCH] fbcon: use default if cursor blink interval is not valid
Hi Scot, > Use the normal cursor blink default interval of 200 ms if > ops->cur_blink_jiffies is not in the range specified in commit > bd63364caa8d. Since invalid values are not used, specific system > initialization timings should not cause lockups. This fixes an issue we're seeing with the ast driver on OpenPOWER machines, thanks! Acked-by: Jeremy Kerr <j...@ozlabs.org> Cheers, Jeremy
Re: [PATCH] fbcon: use default if cursor blink interval is not valid
Hi Scot, > Use the normal cursor blink default interval of 200 ms if > ops->cur_blink_jiffies is not in the range specified in commit > bd63364caa8d. Since invalid values are not used, specific system > initialization timings should not cause lockups. This fixes an issue we're seeing with the ast driver on OpenPOWER machines, thanks! Acked-by: Jeremy Kerr Cheers, Jeremy
Re: [PATCH] drivers/core/of: Add symlink to device-tree from devices with an OF node
Hi Rob, > struct device doesn't have an of_node member if !CONFIG_OF, so we'll > need to disable this block in the preprocessor. Scratch that, I was looking at the wrong header - we do indeed have the of_node available independently of CONFIG_OF, and this makes the logic a little cleaner. Cheers, Jeremy -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] drivers/core/of: Add symlink to device-tree from devices with an OF node
Hi Rob,
>> diff --git a/drivers/base/core.c b/drivers/base/core.c
>> index 20da3ad..8c7b607 100644
>> --- a/drivers/base/core.c
>> +++ b/drivers/base/core.c
>> @@ -493,6 +493,15 @@ static int device_add_attrs(struct device *dev)
>> goto err_remove_dev_groups;
>> }
>>
>> +#ifdef CONFIG_OF
>> + if (dev->of_node) {
>
> if (IS_ENABLED(CONFIG_OF) && dev->of_node)
struct device doesn't have an of_node member if !CONFIG_OF, so we'll
need to disable this block in the preprocessor.
Cheers,
Jeremy
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] drivers/core/of: Add symlink to device-tree from devices with an OF node
Hi Rob,
diff --git a/drivers/base/core.c b/drivers/base/core.c
index 20da3ad..8c7b607 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -493,6 +493,15 @@ static int device_add_attrs(struct device *dev)
goto err_remove_dev_groups;
}
+#ifdef CONFIG_OF
+ if (dev-of_node) {
if (IS_ENABLED(CONFIG_OF) dev-of_node)
struct device doesn't have an of_node member if !CONFIG_OF, so we'll
need to disable this block in the preprocessor.
Cheers,
Jeremy
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] drivers/core/of: Add symlink to device-tree from devices with an OF node
Hi Rob, struct device doesn't have an of_node member if !CONFIG_OF, so we'll need to disable this block in the preprocessor. Scratch that, I was looking at the wrong header - we do indeed have the of_node available independently of CONFIG_OF, and this makes the logic a little cleaner. Cheers, Jeremy -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 02/15] powerpc/cell: Move data segment faulting code out of cell platform
Hi Mikey & Ian,
> __spu_trap_data_seg() currently contains code to determine the VSID and ESID
> required for a particular EA and mm struct.
>
> This code is generically useful for other co-processors. This moves the code
> of the cell platform so it can be used by other powerpc code.
OK, nice.
> +
> +int copro_data_segment(struct mm_struct *mm, u64 ea, u64 *esid, u64 *vsid)
> +{
> + int psize, ssize;
> +
> + *esid = (ea & ESID_MASK) | SLB_ESID_V;
> +
> + switch (REGION_ID(ea)) {
> + case USER_REGION_ID:
> + pr_devel("copro_data_segment: 0x%llx -- USER_REGION_ID\n", ea);
> +#ifdef CONFIG_PPC_MM_SLICES
> + psize = get_slice_psize(mm, ea);
> +#else
> + psize = mm->context.user_psize;
> +#endif
> + ssize = user_segment_size(ea);
> + *vsid = (get_vsid(mm->context.id, ea, ssize)
> + << slb_vsid_shift(ssize)) | SLB_VSID_USER
> + | (ssize == MMU_SEGSIZE_1T ? SLB_VSID_B_1T : 0);
> + break;
> + case VMALLOC_REGION_ID:
> + pr_devel("copro_data_segment: 0x%llx -- VMALLOC_REGION_ID\n",
> ea);
> + if (ea < VMALLOC_END)
> + psize = mmu_vmalloc_psize;
> + else
> + psize = mmu_io_psize;
> + *vsid = (get_kernel_vsid(ea, mmu_kernel_ssize)
> + << SLB_VSID_SHIFT) | SLB_VSID_KERNEL
> + | (mmu_kernel_ssize == MMU_SEGSIZE_1T ? SLB_VSID_B_1T :
> 0);
> + break;
> + case KERNEL_REGION_ID:
> + pr_devel("copro_data_segment: 0x%llx -- KERNEL_REGION_ID\n",
> ea);
> + psize = mmu_linear_psize;
> + *vsid = (get_kernel_vsid(ea, mmu_kernel_ssize)
> + << SLB_VSID_SHIFT) | SLB_VSID_KERNEL
> + | (mmu_kernel_ssize == MMU_SEGSIZE_1T ? SLB_VSID_B_1T :
> 0);
> + break;
> + default:
> + /* Future: support kernel segments so that drivers can use the
> + * CoProcessors */
> + pr_debug("invalid region access at %016llx\n", ea);
> + return 1;
> + }
> + *vsid |= mmu_psize_defs[psize].sllp;
A bit of a nitpick, but how about you remove the repeated:
| ( == MMU_SEGSIZE_1T ? SLB_VSID_B_1T : 0)
then set ssize in each of the switch cases (like we do with psize), and
or-in the VSID_B_1T bit at the end:
*vsid |= mmu_psize_defs[psize].sllp
| (ssize == MMU_SEGSIZE_1T ? SLB_VSID_B_1T : 0);
Otherwise, looks good to me.
Cheers,
Jeremy
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 01/15] powerpc/cell: Move spu_handle_mm_fault() out of cell platform
Hi Mikey & Ian,
> Currently spu_handle_mm_fault() is in the cell platform.
>
> This code is generically useful for other non-cell co-processors on powerpc.
>
> This patch moves this function out of the cell platform into arch/powerpc/mm
> so
> that others may use it.
Makes sense.
Acked-by: Jeremy Kerr
> @@ -58,12 +56,12 @@ int spu_handle_mm_fault(struct mm_struct *mm, unsigned
> long ea,
> goto out_unlock;
> }
>
> - is_write = dsisr & MFC_DSISR_ACCESS_PUT;
> + is_write = dsisr & DSISR_ISSTORE;
> if (is_write) {
> if (!(vma->vm_flags & VM_WRITE))
> goto out_unlock;
> } else {
> - if (dsisr & MFC_DSISR_ACCESS_DENIED)
> + if (dsisr & DSISR_PROTFAULT)
> goto out_unlock;
> if (!(vma->vm_flags & (VM_READ | VM_EXEC)))
> goto out_unlock;
Consistent DSISR encodings? woot! :)
Cheers,
Jeremy
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 01/15] powerpc/cell: Move spu_handle_mm_fault() out of cell platform
Hi Mikey Ian,
Currently spu_handle_mm_fault() is in the cell platform.
This code is generically useful for other non-cell co-processors on powerpc.
This patch moves this function out of the cell platform into arch/powerpc/mm
so
that others may use it.
Makes sense.
Acked-by: Jeremy Kerr j...@ozlabs.org
@@ -58,12 +56,12 @@ int spu_handle_mm_fault(struct mm_struct *mm, unsigned
long ea,
goto out_unlock;
}
- is_write = dsisr MFC_DSISR_ACCESS_PUT;
+ is_write = dsisr DSISR_ISSTORE;
if (is_write) {
if (!(vma-vm_flags VM_WRITE))
goto out_unlock;
} else {
- if (dsisr MFC_DSISR_ACCESS_DENIED)
+ if (dsisr DSISR_PROTFAULT)
goto out_unlock;
if (!(vma-vm_flags (VM_READ | VM_EXEC)))
goto out_unlock;
Consistent DSISR encodings? woot! :)
Cheers,
Jeremy
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 02/15] powerpc/cell: Move data segment faulting code out of cell platform
Hi Mikey Ian,
__spu_trap_data_seg() currently contains code to determine the VSID and ESID
required for a particular EA and mm struct.
This code is generically useful for other co-processors. This moves the code
of the cell platform so it can be used by other powerpc code.
OK, nice.
+
+int copro_data_segment(struct mm_struct *mm, u64 ea, u64 *esid, u64 *vsid)
+{
+ int psize, ssize;
+
+ *esid = (ea ESID_MASK) | SLB_ESID_V;
+
+ switch (REGION_ID(ea)) {
+ case USER_REGION_ID:
+ pr_devel(copro_data_segment: 0x%llx -- USER_REGION_ID\n, ea);
+#ifdef CONFIG_PPC_MM_SLICES
+ psize = get_slice_psize(mm, ea);
+#else
+ psize = mm-context.user_psize;
+#endif
+ ssize = user_segment_size(ea);
+ *vsid = (get_vsid(mm-context.id, ea, ssize)
+ slb_vsid_shift(ssize)) | SLB_VSID_USER
+ | (ssize == MMU_SEGSIZE_1T ? SLB_VSID_B_1T : 0);
+ break;
+ case VMALLOC_REGION_ID:
+ pr_devel(copro_data_segment: 0x%llx -- VMALLOC_REGION_ID\n,
ea);
+ if (ea VMALLOC_END)
+ psize = mmu_vmalloc_psize;
+ else
+ psize = mmu_io_psize;
+ *vsid = (get_kernel_vsid(ea, mmu_kernel_ssize)
+ SLB_VSID_SHIFT) | SLB_VSID_KERNEL
+ | (mmu_kernel_ssize == MMU_SEGSIZE_1T ? SLB_VSID_B_1T :
0);
+ break;
+ case KERNEL_REGION_ID:
+ pr_devel(copro_data_segment: 0x%llx -- KERNEL_REGION_ID\n,
ea);
+ psize = mmu_linear_psize;
+ *vsid = (get_kernel_vsid(ea, mmu_kernel_ssize)
+ SLB_VSID_SHIFT) | SLB_VSID_KERNEL
+ | (mmu_kernel_ssize == MMU_SEGSIZE_1T ? SLB_VSID_B_1T :
0);
+ break;
+ default:
+ /* Future: support kernel segments so that drivers can use the
+ * CoProcessors */
+ pr_debug(invalid region access at %016llx\n, ea);
+ return 1;
+ }
+ *vsid |= mmu_psize_defs[psize].sllp;
A bit of a nitpick, but how about you remove the repeated:
| (size == MMU_SEGSIZE_1T ? SLB_VSID_B_1T : 0)
then set ssize in each of the switch cases (like we do with psize), and
or-in the VSID_B_1T bit at the end:
*vsid |= mmu_psize_defs[psize].sllp
| (ssize == MMU_SEGSIZE_1T ? SLB_VSID_B_1T : 0);
Otherwise, looks good to me.
Cheers,
Jeremy
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[tip:sched/core] powerpc/spufs: Remove MAX_USER_PRIO define
Commit-ID: 74b8af7837fa55c020e2ad1b34a6b10dfe25a9b1
Gitweb: http://git.kernel.org/tip/74b8af7837fa55c020e2ad1b34a6b10dfe25a9b1
Author: Jeremy Kerr
AuthorDate: Tue, 11 Feb 2014 14:05:17 +0800
Committer: Ingo Molnar
CommitDate: Tue, 11 Feb 2014 09:58:33 +0100
powerpc/spufs: Remove MAX_USER_PRIO define
Current ppc64_defconfig fails with:
arch/powerpc/platforms/cell/spufs/sched.c:86:0: error: "MAX_USER_PRIO"
redefined [-Werror]
cc1: all warnings being treated as errors
Commit 6b6350f155af ("sched: Expose some macros related to priority")
introduced a generic MAX_USER_PRIO macro to sched/prio.h, which is
causing the conflit. Use that one instead of our own.
Reported-by: Fengguang Wu
Reported-by: Stephen Rothwell
Signed-off-by: Jeremy Kerr
Cc: Dongsheng Yang
Cc: linuxppc-...@lists.ozlabs.org
Link: http://lkml.kernel.org/r/1392098717.689604.970589769393.1.gpush@pablo
Signed-off-by: Ingo Molnar
---
arch/powerpc/platforms/cell/spufs/sched.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/arch/powerpc/platforms/cell/spufs/sched.c
b/arch/powerpc/platforms/cell/spufs/sched.c
index 4931838..4a0a64f 100644
--- a/arch/powerpc/platforms/cell/spufs/sched.c
+++ b/arch/powerpc/platforms/cell/spufs/sched.c
@@ -83,7 +83,6 @@ static struct timer_list spuloadavg_timer;
#define MIN_SPU_TIMESLICE max(5 * HZ / (1000 * SPUSCHED_TICK), 1)
#define DEF_SPU_TIMESLICE (100 * HZ / (1000 * SPUSCHED_TICK))
-#define MAX_USER_PRIO (MAX_PRIO - MAX_RT_PRIO)
#define SCALE_PRIO(x, prio) \
max(x * (MAX_PRIO - prio) / (MAX_USER_PRIO / 2), MIN_SPU_TIMESLICE)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[tip:sched/core] powerpc/spufs: Remove MAX_USER_PRIO define
Commit-ID: 74b8af7837fa55c020e2ad1b34a6b10dfe25a9b1 Gitweb: http://git.kernel.org/tip/74b8af7837fa55c020e2ad1b34a6b10dfe25a9b1 Author: Jeremy Kerr j...@ozlabs.org AuthorDate: Tue, 11 Feb 2014 14:05:17 +0800 Committer: Ingo Molnar mi...@kernel.org CommitDate: Tue, 11 Feb 2014 09:58:33 +0100 powerpc/spufs: Remove MAX_USER_PRIO define Current ppc64_defconfig fails with: arch/powerpc/platforms/cell/spufs/sched.c:86:0: error: MAX_USER_PRIO redefined [-Werror] cc1: all warnings being treated as errors Commit 6b6350f155af (sched: Expose some macros related to priority) introduced a generic MAX_USER_PRIO macro to sched/prio.h, which is causing the conflit. Use that one instead of our own. Reported-by: Fengguang Wu fengguang...@intel.com Reported-by: Stephen Rothwell s...@canb.auug.org.au Signed-off-by: Jeremy Kerr j...@ozlabs.org Cc: Dongsheng Yang yangds.f...@cn.fujitsu.com Cc: linuxppc-...@lists.ozlabs.org Link: http://lkml.kernel.org/r/1392098717.689604.970589769393.1.gpush@pablo Signed-off-by: Ingo Molnar mi...@kernel.org --- arch/powerpc/platforms/cell/spufs/sched.c | 1 - 1 file changed, 1 deletion(-) diff --git a/arch/powerpc/platforms/cell/spufs/sched.c b/arch/powerpc/platforms/cell/spufs/sched.c index 4931838..4a0a64f 100644 --- a/arch/powerpc/platforms/cell/spufs/sched.c +++ b/arch/powerpc/platforms/cell/spufs/sched.c @@ -83,7 +83,6 @@ static struct timer_list spuloadavg_timer; #define MIN_SPU_TIMESLICE max(5 * HZ / (1000 * SPUSCHED_TICK), 1) #define DEF_SPU_TIMESLICE (100 * HZ / (1000 * SPUSCHED_TICK)) -#define MAX_USER_PRIO (MAX_PRIO - MAX_RT_PRIO) #define SCALE_PRIO(x, prio) \ max(x * (MAX_PRIO - prio) / (MAX_USER_PRIO / 2), MIN_SPU_TIMESLICE) -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] powerpc/spufs: Remove MAX_USER_PRIO define
Current ppc64_defconfig fails with: arch/powerpc/platforms/cell/spufs/sched.c:86:0: error: "MAX_USER_PRIO" redefined [-Werror] cc1: all warnings being treated as errors 6b6350f1 introduced a generic MAX_USER_PRIO macro to sched/prio.h, which is causing the conflit. Use that one instead of our own. Signed-off-by: Jeremy Kerr --- Ingo: 6b6350f1 is currently in tip; this fixes a build breakage for spufs --- arch/powerpc/platforms/cell/spufs/sched.c |1 - 1 file changed, 1 deletion(-) diff --git a/arch/powerpc/platforms/cell/spufs/sched.c b/arch/powerpc/platforms/cell/spufs/sched.c index 4931838..4a0a64f 100644 --- a/arch/powerpc/platforms/cell/spufs/sched.c +++ b/arch/powerpc/platforms/cell/spufs/sched.c @@ -83,7 +83,6 @@ static struct timer_list spuloadavg_timer; #define MIN_SPU_TIMESLICE max(5 * HZ / (1000 * SPUSCHED_TICK), 1) #define DEF_SPU_TIMESLICE (100 * HZ / (1000 * SPUSCHED_TICK)) -#define MAX_USER_PRIO (MAX_PRIO - MAX_RT_PRIO) #define SCALE_PRIO(x, prio) \ max(x * (MAX_PRIO - prio) / (MAX_USER_PRIO / 2), MIN_SPU_TIMESLICE) -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] powerpc/spufs: Remove MAX_USER_PRIO define
Current ppc64_defconfig fails with: arch/powerpc/platforms/cell/spufs/sched.c:86:0: error: MAX_USER_PRIO redefined [-Werror] cc1: all warnings being treated as errors 6b6350f1 introduced a generic MAX_USER_PRIO macro to sched/prio.h, which is causing the conflit. Use that one instead of our own. Signed-off-by: Jeremy Kerr j...@ozlabs.org --- Ingo: 6b6350f1 is currently in tip; this fixes a build breakage for spufs --- arch/powerpc/platforms/cell/spufs/sched.c |1 - 1 file changed, 1 deletion(-) diff --git a/arch/powerpc/platforms/cell/spufs/sched.c b/arch/powerpc/platforms/cell/spufs/sched.c index 4931838..4a0a64f 100644 --- a/arch/powerpc/platforms/cell/spufs/sched.c +++ b/arch/powerpc/platforms/cell/spufs/sched.c @@ -83,7 +83,6 @@ static struct timer_list spuloadavg_timer; #define MIN_SPU_TIMESLICE max(5 * HZ / (1000 * SPUSCHED_TICK), 1) #define DEF_SPU_TIMESLICE (100 * HZ / (1000 * SPUSCHED_TICK)) -#define MAX_USER_PRIO (MAX_PRIO - MAX_RT_PRIO) #define SCALE_PRIO(x, prio) \ max(x * (MAX_PRIO - prio) / (MAX_USER_PRIO / 2), MIN_SPU_TIMESLICE) -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: linux-next: build failure after merge of the final tree
Hi all, > Yes, I agree. The other filesystems that take an Opt_uid as well do use > current_user_ns() and not init_user_ns. They also do a uid_valid() > check and fail the mount (or fallback to GLOBAL_ROOT_UID). So I think > that would look like this: Looks good to me. Builds and mounts as expected. Acked-by: Jeremy Kerr Cheers, Jeremy -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: linux-next: build failure after merge of the final tree
Hi all, Yes, I agree. The other filesystems that take an Opt_uid as well do use current_user_ns() and not init_user_ns. They also do a uid_valid() check and fail the mount (or fallback to GLOBAL_ROOT_UID). So I think that would look like this: Looks good to me. Builds and mounts as expected. Acked-by: Jeremy Kerr j...@ozlabs.org Cheers, Jeremy -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [git pull] Please pull powerpc.git merge branch
Hi Linus, > No. The date from the email was > > Date: Fri, 7 Jun 2013 15:42:54 +1000 > > and we want *that* date. Ah, gotchya. So, we now use the original date header (if present) in the mbox views: $ wget -qO - http://patchwork.ozlabs.org/patch/249598/mbox/ | grep ^Date Date: Fri, 7 Jun 2013 15:42:54 +1000 ... for all your data-mining needs. Cheers, Jeremy -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [git pull] Please pull powerpc.git merge branch
Hi Linus, > Is Jeremy the patchwork maintainer? Yep, that's me. > and it turns out that apparently 'patchwork' is just making up random > times, because when you download the email as an mbox, it will turn > this into that corrupt and incorrect > > Date: Thu, 06 Jun 2013 19:42:54 - > > thing which is apparently how you got the wrong timestamp to begin with. We keep all patch dates in UTC, but were generating the Date header incorrectly. Now fixed: $ wget -qO - http://patchwork.ozlabs.org/patch/249598/mbox/ | grep ^Date Date: Fri, 07 Jun 2013 05:42:54 - Commit is at: http://git.ozlabs.org/?p=patchwork;a=commitdiff;h=e7353352 Cheers, Jeremy -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [git pull] Please pull powerpc.git merge branch
Hi Linus, Is Jeremy the patchwork maintainer? Yep, that's me. and it turns out that apparently 'patchwork' is just making up random times, because when you download the email as an mbox, it will turn this into that corrupt and incorrect Date: Thu, 06 Jun 2013 19:42:54 - thing which is apparently how you got the wrong timestamp to begin with. We keep all patch dates in UTC, but were generating the Date header incorrectly. Now fixed: $ wget -qO - http://patchwork.ozlabs.org/patch/249598/mbox/ | grep ^Date Date: Fri, 07 Jun 2013 05:42:54 - Commit is at: http://git.ozlabs.org/?p=patchwork;a=commitdiff;h=e7353352 Cheers, Jeremy -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [git pull] Please pull powerpc.git merge branch
Hi Linus, No. The date from the email was Date: Fri, 7 Jun 2013 15:42:54 +1000 and we want *that* date. Ah, gotchya. So, we now use the original date header (if present) in the mbox views: $ wget -qO - http://patchwork.ozlabs.org/patch/249598/mbox/ | grep ^Date Date: Fri, 7 Jun 2013 15:42:54 +1000 ... for all your data-mining needs. Cheers, Jeremy -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3.8-stable] net: count hw_addr syncs so that unsync works properly.
Hi all, >>> This patch looks like it should be in the 3.8-stable tree, should we apply >>> it? >> >> I queue up networking patches as needed and that queue is >> visible at: >> >> http://patchwork.ozlabs.org/user/bundle/2566/?state=* > > Actually, this bundle is not visible via that link. It appears to be a > public bundle and visible via > http://patchwork.ozlabs.org/bundle/davem/stable/?state=* . I have > insider knowledge :-) Perhaps for public bundles, I should make the private link automatically redirect to the public one? Cheers, Jeremy -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3.8-stable] net: count hw_addr syncs so that unsync works properly.
Hi all, This patch looks like it should be in the 3.8-stable tree, should we apply it? I queue up networking patches as needed and that queue is visible at: http://patchwork.ozlabs.org/user/bundle/2566/?state=* Actually, this bundle is not visible via that link. It appears to be a public bundle and visible via http://patchwork.ozlabs.org/bundle/davem/stable/?state=* . I have insider knowledge :-) Perhaps for public bundles, I should make the private link automatically redirect to the public one? Cheers, Jeremy -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] Documentation: Add a simple doc for selftests
On 08/02/13 07:13, Andrew Morton wrote: > The general ruleset for selftests is: do as much as you can if you're not > root and don't take too long and don't break the build on any > architecture and don't cause the top-level "make run_tests" to fail if > your feature is unconfigured. This change adds a little documentation to the tests under tools/testing/selftests/, based on akpm's explanation. Signed-off-by: Jeremy Kerr --- Documentation/selftests.txt | 43 1 file changed, 43 insertions(+) diff --git a/Documentation/selftests.txt b/Documentation/selftests.txt new file mode 100644 index 000..a00e477 --- /dev/null +++ b/Documentation/selftests.txt @@ -0,0 +1,43 @@ +Linux Kernel Selftests + +The kernel contains a set of "self tests" under the tools/testing/selftests/ +directory. These are intended to be small unit tests to exercise individual +code paths in the kernel. + +Running the selftests += + +To build the tests: + + $ make -C tools/testing/selftests + + +To run the tests: + + $ make -C tools/testing/selftests run_tests + +- note that some tests will require root privileges. + + +To run only tests targetted for a single subsystem: + + $ make -C tools/testing/selftests TARGETS=cpu-hotplug run_tests + +See the top-level tools/testing/selftests/Makefile for the list of all possible +targets. + + +Contributing new tests +== + +In general, the rules for for selftests are + + * Do as much as you can if you're not root; + + * Don't take too long; + + * Don't break the build on any architecture, and + + * Don't cause the top-level "make run_tests" to fail if your feature is + unconfigured. + -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/3 v3] selftests: Add tests for efivarfs
Hi Andrew, Thanks for taking a look at these. @@ -1,4 +1,4 @@ -TARGETS = breakpoints kcmp mqueue vm cpu-hotplug memory-hotplug +TARGETS = breakpoints kcmp mqueue vm cpu-hotplug memory-hotplug efivarfs bah. This sort of Makefile construct is a wonderful source of patch rejects and fixups. I'll covert this to --- a/tools/testing/selftests/Makefile~a +++ a/tools/testing/selftests/Makefile @@ -1,4 +1,11 @@ -TARGETS = breakpoints epoll kcmp mqueue vm cpu-hotplug memory-hotplug efivarfs +TARGETS = breakpoints +TARGETS += epoll +TARGETS += kcmp +TARGETS += mqueue +TARGETS += vm +TARGETS += cpu-hotplug +TARGETS += memory-hotplug +TARGETS += efivarfs Much better, thanks. I'd already had a collision with the epoll tests... I'll do this for now: --- a/tools/testing/selftests/efivarfs/Makefile~selftests-add-tests-for-efivarfs-fix +++ a/tools/testing/selftests/efivarfs/Makefile @@ -6,7 +6,7 @@ test_objs = open-unlink all: $(test_objs) run_tests: all - @./efivarfs.sh || echo "efivarfs selftests: [FAIL]" + @/bin/sh ./efivarfs.sh || echo "efivarfs selftests: [FAIL]" clean: rm -f $(test_objs) but I'm not sure I did it right :( efivarfs.sh requires bash currently, so we'll need to call this explicitly: + @/bin/bash ./efivarfs.sh || echo "efivarfs selftests: [FAIL]" Is this okay? The general ruleset for selftests is: do as much as you can if you're not root and don't take too long and don't break the build on any architecture and don't cause the top-level "make run_tests" to fail if your feature is unconfigured. Ah, good stuff to know. I'll send a patch adding this info to Documentation/ too. Does this code pass all that? It should, yes: * all test requires root at present, as all efivarfs files are only writable by root * the built binaries doesn't use anything more than basic C, so should build fine wherever we have gcc. * efivarfs.sh will skip all tests if efivarfs is not mounted However, the tests expose a bug at the moment, so run_tests will fail. Matt will have that fixed soon though :) Cheers, Jeremy -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/3 v3] selftests: Add tests for efivarfs
Hi Andrew, Thanks for taking a look at these. @@ -1,4 +1,4 @@ -TARGETS = breakpoints kcmp mqueue vm cpu-hotplug memory-hotplug +TARGETS = breakpoints kcmp mqueue vm cpu-hotplug memory-hotplug efivarfs bah. This sort of Makefile construct is a wonderful source of patch rejects and fixups. I'll covert this to --- a/tools/testing/selftests/Makefile~a +++ a/tools/testing/selftests/Makefile @@ -1,4 +1,11 @@ -TARGETS = breakpoints epoll kcmp mqueue vm cpu-hotplug memory-hotplug efivarfs +TARGETS = breakpoints +TARGETS += epoll +TARGETS += kcmp +TARGETS += mqueue +TARGETS += vm +TARGETS += cpu-hotplug +TARGETS += memory-hotplug +TARGETS += efivarfs Much better, thanks. I'd already had a collision with the epoll tests... I'll do this for now: --- a/tools/testing/selftests/efivarfs/Makefile~selftests-add-tests-for-efivarfs-fix +++ a/tools/testing/selftests/efivarfs/Makefile @@ -6,7 +6,7 @@ test_objs = open-unlink all: $(test_objs) run_tests: all - @./efivarfs.sh || echo efivarfs selftests: [FAIL] + @/bin/sh ./efivarfs.sh || echo efivarfs selftests: [FAIL] clean: rm -f $(test_objs) but I'm not sure I did it right :( efivarfs.sh requires bash currently, so we'll need to call this explicitly: + @/bin/bash ./efivarfs.sh || echo efivarfs selftests: [FAIL] Is this okay? The general ruleset for selftests is: do as much as you can if you're not root and don't take too long and don't break the build on any architecture and don't cause the top-level make run_tests to fail if your feature is unconfigured. Ah, good stuff to know. I'll send a patch adding this info to Documentation/ too. Does this code pass all that? It should, yes: * all test requires root at present, as all efivarfs files are only writable by root * the built binaries doesn't use anything more than basic C, so should build fine wherever we have gcc. * efivarfs.sh will skip all tests if efivarfs is not mounted However, the tests expose a bug at the moment, so run_tests will fail. Matt will have that fixed soon though :) Cheers, Jeremy -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] Documentation: Add a simple doc for selftests
On 08/02/13 07:13, Andrew Morton wrote: The general ruleset for selftests is: do as much as you can if you're not root and don't take too long and don't break the build on any architecture and don't cause the top-level make run_tests to fail if your feature is unconfigured. This change adds a little documentation to the tests under tools/testing/selftests/, based on akpm's explanation. Signed-off-by: Jeremy Kerr j...@ozlabs.org --- Documentation/selftests.txt | 43 1 file changed, 43 insertions(+) diff --git a/Documentation/selftests.txt b/Documentation/selftests.txt new file mode 100644 index 000..a00e477 --- /dev/null +++ b/Documentation/selftests.txt @@ -0,0 +1,43 @@ +Linux Kernel Selftests + +The kernel contains a set of self tests under the tools/testing/selftests/ +directory. These are intended to be small unit tests to exercise individual +code paths in the kernel. + +Running the selftests += + +To build the tests: + + $ make -C tools/testing/selftests + + +To run the tests: + + $ make -C tools/testing/selftests run_tests + +- note that some tests will require root privileges. + + +To run only tests targetted for a single subsystem: + + $ make -C tools/testing/selftests TARGETS=cpu-hotplug run_tests + +See the top-level tools/testing/selftests/Makefile for the list of all possible +targets. + + +Contributing new tests +== + +In general, the rules for for selftests are + + * Do as much as you can if you're not root; + + * Don't take too long; + + * Don't break the build on any architecture, and + + * Don't cause the top-level make run_tests to fail if your feature is + unconfigured. + -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 3/3 v3] selftests/efivarfs: Add create-read test
Test that reads from a newly-created efivarfs file (with no data
written) will return EOF.
Signed-off-by: Jeremy Kerr
---
tools/testing/selftests/efivarfs/Makefile |2
tools/testing/selftests/efivarfs/create-read.c | 38 +
tools/testing/selftests/efivarfs/efivarfs.sh |7 +++
3 files changed, 46 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/efivarfs/Makefile
b/tools/testing/selftests/efivarfs/Makefile
index 1a943ee..b4a7aca 100644
--- a/tools/testing/selftests/efivarfs/Makefile
+++ b/tools/testing/selftests/efivarfs/Makefile
@@ -1,7 +1,7 @@
CC = $(CROSS_COMPILE)gcc
CFLAGS = -Wall
-test_objs = open-unlink
+test_objs = open-unlink create-read
all: $(test_objs)
diff --git a/tools/testing/selftests/efivarfs/create-read.c
b/tools/testing/selftests/efivarfs/create-read.c
new file mode 100644
index 000..7feef18
--- /dev/null
+++ b/tools/testing/selftests/efivarfs/create-read.c
@@ -0,0 +1,38 @@
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+int main(int argc, char **argv)
+{
+ const char *path;
+ char buf[4];
+ int fd, rc;
+
+ if (argc < 2) {
+ fprintf(stderr, "usage: %s \n", argv[0]);
+ return EXIT_FAILURE;
+ }
+
+ path = argv[1];
+
+ /* create a test variable */
+ fd = open(path, O_RDWR | O_CREAT, 0600);
+ if (fd < 0) {
+ perror("open(O_WRONLY)");
+ return EXIT_FAILURE;
+ }
+
+ rc = read(fd, buf, sizeof(buf));
+ if (rc != 0) {
+ fprintf(stderr, "Reading a new var should return EOF\n");
+ return EXIT_FAILURE;
+ }
+
+ return EXIT_SUCCESS;
+}
diff --git a/tools/testing/selftests/efivarfs/efivarfs.sh
b/tools/testing/selftests/efivarfs/efivarfs.sh
index 3af4b37..880cdd5 100755
--- a/tools/testing/selftests/efivarfs/efivarfs.sh
+++ b/tools/testing/selftests/efivarfs/efivarfs.sh
@@ -70,6 +70,12 @@ test_create_empty()
fi
}
+test_create_read()
+{
+ local file=$efivarfs_mount/$FUNCNAME-$test_guid
+ ./create-read $file
+}
+
test_delete()
{
local attrs='\x07\x00\x00\x00'
@@ -125,6 +131,7 @@ rc=0
run_test test_create
run_test test_create_empty
+run_test test_create_read
run_test test_delete
run_test test_zero_size_delete
run_test test_open_unlink
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 0/3 v3] selftests: Add efivarfs tests
Hi all, The following patches add a small set of tests to the tools/testing/selftests directory. These cover some of the basic functionality of efivarfs. Comments welcome - changes 2 and 3 cover some behaviour that Matt and I have discussed, but I'd appreciate any wider comment on the semantics covered by these. Cheers, Jeremy -- v2: Remove qemu check, add a couple of tests v3: Change expected empty read() behaviour to return EOF --- Jeremy Kerr (3): selftests: Add tests for efivarfs selftests/efivarfs: Add empty file creation test selftests/efivarfs: Add create-read test -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/3 v3] selftests: Add tests for efivarfs
This change adds a few initial efivarfs tests to the
tools/testing/selftests directory.
The open-unlink test is based on code from
Lingzhu Xiang .
Signed-off-by: Jeremy Kerr
---
tools/testing/selftests/Makefile |2
tools/testing/selftests/efivarfs/Makefile | 12 +
tools/testing/selftests/efivarfs/efivarfs.sh | 119 +
tools/testing/selftests/efivarfs/open-unlink.c | 63 +
4 files changed, 195 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
index 85baf11..dee19dd 100644
--- a/tools/testing/selftests/Makefile
+++ b/tools/testing/selftests/Makefile
@@ -1,4 +1,4 @@
-TARGETS = breakpoints kcmp mqueue vm cpu-hotplug memory-hotplug
+TARGETS = breakpoints kcmp mqueue vm cpu-hotplug memory-hotplug efivarfs
all:
for TARGET in $(TARGETS); do \
diff --git a/tools/testing/selftests/efivarfs/Makefile
b/tools/testing/selftests/efivarfs/Makefile
new file mode 100644
index 000..1a943ee
--- /dev/null
+++ b/tools/testing/selftests/efivarfs/Makefile
@@ -0,0 +1,12 @@
+CC = $(CROSS_COMPILE)gcc
+CFLAGS = -Wall
+
+test_objs = open-unlink
+
+all: $(test_objs)
+
+run_tests: all
+ @./efivarfs.sh || echo "efivarfs selftests: [FAIL]"
+
+clean:
+ rm -f $(test_objs)
diff --git a/tools/testing/selftests/efivarfs/efivarfs.sh
b/tools/testing/selftests/efivarfs/efivarfs.sh
new file mode 100755
index 000..e8c0d27
--- /dev/null
+++ b/tools/testing/selftests/efivarfs/efivarfs.sh
@@ -0,0 +1,119 @@
+#!/bin/bash
+
+efivarfs_mount=/sys/firmware/efi/efivars
+test_guid=210be57c-9849-4fc7-a635-e6382d1aec27
+
+check_prereqs()
+{
+ local msg="skip all tests:"
+
+ if [ $UID != 0 ]; then
+ echo $msg must be run as root >&2
+ exit 0
+ fi
+
+ if ! grep -q "^\S\+ $efivarfs_mount efivarfs" /proc/mounts; then
+ echo $msg efivarfs is not mounted on $efivarfs_mount >&2
+ exit 0
+ fi
+}
+
+run_test()
+{
+ local test="$1"
+
+ echo ""
+ echo "running $test"
+ echo ""
+
+ if [ "$(type -t $test)" = 'function' ]; then
+ ( $test )
+ else
+ ( ./$test )
+ fi
+
+ if [ $? -ne 0 ]; then
+ echo " [FAIL]"
+ rc=1
+ else
+ echo " [PASS]"
+ fi
+}
+
+test_create()
+{
+ local attrs='\x07\x00\x00\x00'
+ local file=$efivarfs_mount/$FUNCNAME-$test_guid
+
+ printf "$attrs\x00" > $file
+
+ if [ ! -e $file ]; then
+ echo "$file couldn't be created" >&2
+ exit 1
+ fi
+
+ if [ $(stat -c %s $file) -ne 5 ]; then
+ echo "$file has invalid size" >&2
+ exit 1
+ fi
+}
+
+test_delete()
+{
+ local attrs='\x07\x00\x00\x00'
+ local file=$efivarfs_mount/$FUNCNAME-$test_guid
+
+ printf "$attrs\x00" > $file
+
+ if [ ! -e $file ]; then
+ echo "$file couldn't be created" >&2
+ exit 1
+ fi
+
+ rm $file
+
+ if [ -e $file ]; then
+ echo "$file couldn't be deleted" >&2
+ exit 1
+ fi
+
+}
+
+# test that we can remove a variable by issuing a write with only
+# attributes specified
+test_zero_size_delete()
+{
+ local attrs='\x07\x00\x00\x00'
+ local file=$efivarfs_mount/$FUNCNAME-$test_guid
+
+ printf "$attrs\x00" > $file
+
+ if [ ! -e $file ]; then
+ echo "$file does not exist" >&2
+ exit 1
+ fi
+
+ printf "$attrs" > $file
+
+ if [ -e $file ]; then
+ echo "$file should have been deleted" >&2
+ exit 1
+ fi
+}
+
+test_open_unlink()
+{
+ local file=$efivarfs_mount/$FUNCNAME-$test_guid
+ ./open-unlink $file
+}
+
+check_prereqs
+
+rc=0
+
+run_test test_create
+run_test test_delete
+run_test test_zero_size_delete
+run_test test_open_unlink
+
+exit $rc
diff --git a/tools/testing/selftests/efivarfs/open-unlink.c
b/tools/testing/selftests/efivarfs/open-unlink.c
new file mode 100644
index 000..8c07644
--- /dev/null
+++ b/tools/testing/selftests/efivarfs/open-unlink.c
@@ -0,0 +1,63 @@
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+int main(int argc, char **argv)
+{
+ const char *path;
+ char buf[5];
+ int fd, rc;
+
+ if (argc < 2) {
+ fprintf(stderr, "usage: %s \n", argv[0]);
+ return EXIT_FAILURE;
+ }
+
+ path = argv[1];
+
+ /* attributes: EFI_VARIABLE_NON_VOLATILE |
+* EFI_VARIABLE
[PATCH 2/3 v3] selftests/efivarfs: Add empty file creation test
Signed-off-by: Jeremy Kerr
---
tools/testing/selftests/efivarfs/efivarfs.sh | 13 +
1 file changed, 13 insertions(+)
diff --git a/tools/testing/selftests/efivarfs/efivarfs.sh
b/tools/testing/selftests/efivarfs/efivarfs.sh
index e8c0d27..3af4b37 100755
--- a/tools/testing/selftests/efivarfs/efivarfs.sh
+++ b/tools/testing/selftests/efivarfs/efivarfs.sh
@@ -58,6 +58,18 @@ test_create()
fi
}
+test_create_empty()
+{
+ local file=$efivarfs_mount/$FUNCNAME-$test_guid
+
+ : > $file
+
+ if [ ! -e $file ]; then
+ echo "$file can not be created without writing" >&2
+ exit 1
+ fi
+}
+
test_delete()
{
local attrs='\x07\x00\x00\x00'
@@ -112,6 +124,7 @@ check_prereqs
rc=0
run_test test_create
+run_test test_create_empty
run_test test_delete
run_test test_zero_size_delete
run_test test_open_unlink
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
