Message ID | 20200520181707.9235-1-eajames@linux.ibm.com |
---|---|
State | Accepted, archived |
Headers | show |
Series | [linux,dev-5.4] fsi: core: Set slave local bus ownership during init | expand |
On Wed, 20 May 2020 at 18:17, Eddie James <eajames@linux.ibm.com> wrote: > > The driver ought to claim local bus ownership of the slave it's > communicating with. I am not familiar with this register's purpose so feel free to explain the details. Should we also "un-claim" ownership at any point? > > Signed-off-by: Eddie James <eajames@linux.ibm.com> > --- > drivers/fsi/fsi-core.c | 16 +++++++++++++++- > 1 file changed, 15 insertions(+), 1 deletion(-) > > diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c > index 8244da8a7241..48424b672295 100644 > --- a/drivers/fsi/fsi-core.c > +++ b/drivers/fsi/fsi-core.c > @@ -50,6 +50,7 @@ static const int engine_page_size = 0x400; > #define FSI_SMODE 0x0 /* R/W: Mode register */ > #define FSI_SISC 0x8 /* R/W: Interrupt condition */ > #define FSI_SSTAT 0x14 /* R : Slave status */ > +#define FSI_SLBUS 0x30 /* W : LBUS Ownership */ > #define FSI_LLMODE 0x100 /* R/W: Link layer mode register */ > > /* > @@ -66,6 +67,11 @@ static const int engine_page_size = 0x400; > #define FSI_SMODE_LBCRR_SHIFT 8 /* Clk ratio shift */ > #define FSI_SMODE_LBCRR_MASK 0xf /* Clk ratio mask */ > > +/* > + * SLBUS fields > + */ > +#define FSI_SLBUS_FORCE 0x80000000 /* Force LBUS ownership */ > + > /* > * LLMODE fields > */ > @@ -981,7 +987,7 @@ static int fsi_slave_init(struct fsi_master *master, int link, uint8_t id) > uint32_t cfam_id; > struct fsi_slave *slave; > uint8_t crc; > - __be32 data, llmode; > + __be32 data, llmode, slbus; > int rc; > > /* Currently, we only support single slaves on a link, and use the > @@ -1052,6 +1058,14 @@ static int fsi_slave_init(struct fsi_master *master, int link, uint8_t id) > > } > > + slbus = cpu_to_be32(FSI_SLBUS_FORCE); > + rc = fsi_master_write(master, link, id, FSI_SLAVE_BASE + FSI_SLBUS, > + &slbus, sizeof(slbus)); > + if (rc) > + dev_warn(&master->dev, > + "can't set slbus on slave:%02x:%02x %d\n", link, id, > + rc); > + > rc = fsi_slave_set_smode(slave); > if (rc) { > dev_warn(&master->dev, > -- > 2.24.0 >
On 6/1/20 9:05 PM, Joel Stanley wrote: > On Wed, 20 May 2020 at 18:17, Eddie James <eajames@linux.ibm.com> wrote: >> The driver ought to claim local bus ownership of the slave it's >> communicating with. > I am not familiar with this register's purpose so feel free to explain > the details. Sure, I'll add a comment. I don't really understand the details either, but I think this is effectively for multi-master setups. The slave (in theory) will deny access to masters who try to access the CFAM address space but who don't "own" the bus. > > Should we also "un-claim" ownership at any point? Well, the driver doesn't seem to perform any other teardown, so I don't think so. Also I'm not aware of any multi-master setup using this driver so it shouldn't actually matter. Also, the hardware doesn't seem to enforce this despite being required in the specification... Thanks, Eddie > >> Signed-off-by: Eddie James <eajames@linux.ibm.com> >> --- >> drivers/fsi/fsi-core.c | 16 +++++++++++++++- >> 1 file changed, 15 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c >> index 8244da8a7241..48424b672295 100644 >> --- a/drivers/fsi/fsi-core.c >> +++ b/drivers/fsi/fsi-core.c >> @@ -50,6 +50,7 @@ static const int engine_page_size = 0x400; >> #define FSI_SMODE 0x0 /* R/W: Mode register */ >> #define FSI_SISC 0x8 /* R/W: Interrupt condition */ >> #define FSI_SSTAT 0x14 /* R : Slave status */ >> +#define FSI_SLBUS 0x30 /* W : LBUS Ownership */ >> #define FSI_LLMODE 0x100 /* R/W: Link layer mode register */ >> >> /* >> @@ -66,6 +67,11 @@ static const int engine_page_size = 0x400; >> #define FSI_SMODE_LBCRR_SHIFT 8 /* Clk ratio shift */ >> #define FSI_SMODE_LBCRR_MASK 0xf /* Clk ratio mask */ >> >> +/* >> + * SLBUS fields >> + */ >> +#define FSI_SLBUS_FORCE 0x80000000 /* Force LBUS ownership */ >> + >> /* >> * LLMODE fields >> */ >> @@ -981,7 +987,7 @@ static int fsi_slave_init(struct fsi_master *master, int link, uint8_t id) >> uint32_t cfam_id; >> struct fsi_slave *slave; >> uint8_t crc; >> - __be32 data, llmode; >> + __be32 data, llmode, slbus; >> int rc; >> >> /* Currently, we only support single slaves on a link, and use the >> @@ -1052,6 +1058,14 @@ static int fsi_slave_init(struct fsi_master *master, int link, uint8_t id) >> >> } >> >> + slbus = cpu_to_be32(FSI_SLBUS_FORCE); >> + rc = fsi_master_write(master, link, id, FSI_SLAVE_BASE + FSI_SLBUS, >> + &slbus, sizeof(slbus)); >> + if (rc) >> + dev_warn(&master->dev, >> + "can't set slbus on slave:%02x:%02x %d\n", link, id, >> + rc); >> + >> rc = fsi_slave_set_smode(slave); >> if (rc) { >> dev_warn(&master->dev, >> -- >> 2.24.0 >>
diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c index 8244da8a7241..48424b672295 100644 --- a/drivers/fsi/fsi-core.c +++ b/drivers/fsi/fsi-core.c @@ -50,6 +50,7 @@ static const int engine_page_size = 0x400; #define FSI_SMODE 0x0 /* R/W: Mode register */ #define FSI_SISC 0x8 /* R/W: Interrupt condition */ #define FSI_SSTAT 0x14 /* R : Slave status */ +#define FSI_SLBUS 0x30 /* W : LBUS Ownership */ #define FSI_LLMODE 0x100 /* R/W: Link layer mode register */ /* @@ -66,6 +67,11 @@ static const int engine_page_size = 0x400; #define FSI_SMODE_LBCRR_SHIFT 8 /* Clk ratio shift */ #define FSI_SMODE_LBCRR_MASK 0xf /* Clk ratio mask */ +/* + * SLBUS fields + */ +#define FSI_SLBUS_FORCE 0x80000000 /* Force LBUS ownership */ + /* * LLMODE fields */ @@ -981,7 +987,7 @@ static int fsi_slave_init(struct fsi_master *master, int link, uint8_t id) uint32_t cfam_id; struct fsi_slave *slave; uint8_t crc; - __be32 data, llmode; + __be32 data, llmode, slbus; int rc; /* Currently, we only support single slaves on a link, and use the @@ -1052,6 +1058,14 @@ static int fsi_slave_init(struct fsi_master *master, int link, uint8_t id) } + slbus = cpu_to_be32(FSI_SLBUS_FORCE); + rc = fsi_master_write(master, link, id, FSI_SLAVE_BASE + FSI_SLBUS, + &slbus, sizeof(slbus)); + if (rc) + dev_warn(&master->dev, + "can't set slbus on slave:%02x:%02x %d\n", link, id, + rc); + rc = fsi_slave_set_smode(slave); if (rc) { dev_warn(&master->dev,
The driver ought to claim local bus ownership of the slave it's communicating with. Signed-off-by: Eddie James <eajames@linux.ibm.com> --- drivers/fsi/fsi-core.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-)