| Message ID | 20170221215032.79282-7-cbostic@linux.vnet.ibm.com |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
Hi Chris, > @@ -255,6 +263,10 @@ int hub_master_break(struct fsi_master *master, int linkno) > uint32_t break_offset = 0x4; /* hw workaround */ > uint32_t addr; > > + /* config table lists 2 entries per hub link */ > + if (linkno >= (master->n_links / 2)) > + return -ENODEV; > + master->n_links should represent the actual number of links, rather than reflect what's in the config table. [besides, don't we do the / 2 during the config table parse anyway?] > command = FSI_BREAK; > addr = (linkno * FSI_MASTER_HUB_LINK_SIZE) + hub->base; > return fsi_slave_write(hub->slave, addr + break_offset, &command, > @@ -641,6 +653,28 @@ static int fsi_master_break(struct fsi_master *master, int link) > > void fsi_master_handle_error(struct fsi_master *master, uint32_t addr) > { > + uint32_t smode = FSI_SLAVE_SMODE_DFLT; > + > + /* Avoid nested error handling */ > + if (test_and_set_bit(FSI_IN_ERR_CLEANUP, &fsi_state)) > + return; Do we have any other bits of state to represent? If not, just use a plain atomic_t rather than a set of flags. Cheers, Jeremy
On 2/22/17 3:55 PM, Jeremy Kerr wrote: > Hi Chris, > >> @@ -255,6 +263,10 @@ int hub_master_break(struct fsi_master *master, int linkno) >> uint32_t break_offset = 0x4; /* hw workaround */ >> uint32_t addr; >> >> + /* config table lists 2 entries per hub link */ >> + if (linkno >= (master->n_links / 2)) >> + return -ENODEV; >> + > master->n_links should represent the actual number of links, rather than > reflect what's in the config table. > > [besides, don't we do the / 2 during the config table parse anyway?] Hi Jeremy, Yes that's true, its set in scan so no need for the /2 here - will fix. >> command = FSI_BREAK; >> addr = (linkno * FSI_MASTER_HUB_LINK_SIZE) + hub->base; >> return fsi_slave_write(hub->slave, addr + break_offset, &command, >> @@ -641,6 +653,28 @@ static int fsi_master_break(struct fsi_master *master, int link) >> >> void fsi_master_handle_error(struct fsi_master *master, uint32_t addr) >> { >> + uint32_t smode = FSI_SLAVE_SMODE_DFLT; >> + >> + /* Avoid nested error handling */ >> + if (test_and_set_bit(FSI_IN_ERR_CLEANUP, &fsi_state)) >> + return; > Do we have any other bits of state to represent? If not, just use a > plain atomic_t rather than a set of flags. No others at this point, will change to an atomic_t Thanks, Chris > Cheers, > > > Jeremy >
diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c index 428675f..b660d33 100644 --- a/drivers/fsi/fsi-core.c +++ b/drivers/fsi/fsi-core.c @@ -21,6 +21,7 @@ #include <linux/slab.h> #include <linux/jiffies.h> #include <linux/delay.h> +#include <linux/bitops.h> #include "fsi-master.h" @@ -42,6 +43,9 @@ #define FSI_PEEK_BASE 0x410 #define FSI_SLAVE_BASE 0x800 +#define FSI_HUB_CONTROL 0x3400 + +#define FSI_SLAVE_SMODE_DFLT 0xa0ff0100 #define FSI_IPOLL_PERIOD msecs_to_jiffies(fsi_ipoll_period_ms) @@ -54,6 +58,10 @@ static const int engine_page_size = 0x400; static struct task_struct *master_ipoll; static unsigned int fsi_ipoll_period_ms = 100; +static unsigned long fsi_state; + +/* state flags */ +#define FSI_IN_ERR_CLEANUP 0 static DEFINE_IDA(master_ida); @@ -255,6 +263,10 @@ int hub_master_break(struct fsi_master *master, int linkno) uint32_t break_offset = 0x4; /* hw workaround */ uint32_t addr; + /* config table lists 2 entries per hub link */ + if (linkno >= (master->n_links / 2)) + return -ENODEV; + command = FSI_BREAK; addr = (linkno * FSI_MASTER_HUB_LINK_SIZE) + hub->base; return fsi_slave_write(hub->slave, addr + break_offset, &command, @@ -641,6 +653,28 @@ static int fsi_master_break(struct fsi_master *master, int link) void fsi_master_handle_error(struct fsi_master *master, uint32_t addr) { + uint32_t smode = FSI_SLAVE_SMODE_DFLT; + + /* Avoid nested error handling */ + if (test_and_set_bit(FSI_IN_ERR_CLEANUP, &fsi_state)) + return; + + fsi_master_break(master, 0); + udelay(200); + master->write(master, 0, 0, FSI_SLAVE_BASE + FSI_SMODE, &smode, + sizeof(smode)); + smode = FSI_MRESB_RST_GEN | FSI_MRESB_RST_ERR; + master->write(master, 0, 0, FSI_HUB_CONTROL + FSI_MRESB0, &smode, + sizeof(smode)); + + if (addr > FSI_HUB_LINK_OFFSET) { + smode = FSI_BREAK; + master->write(master, 0, 0, 0x100004, &smode, sizeof(smode)); + smode = FSI_SLAVE_SMODE_DFLT; + master->write(master, 0, 0, 0x100800, &smode, sizeof(smode)); + } + + clear_bit(FSI_IN_ERR_CLEANUP, &fsi_state); } static int fsi_master_scan(struct fsi_master *master)
Define the bus error cleanup method for primary and hub links. Signed-off-by: Christopher Bostic <cbostic@linux.vnet.ibm.com> --- v2: Change to atomic operation when checking for nested error handling. Move slave access details out of gpio master and into the core. Only allow hub link 1 scans to cut down on scan bus errors. --- drivers/fsi/fsi-core.c | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+)