All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH linux dev-5.4] fsi: core: Set slave local bus ownership during init
@ 2020-05-20 18:17 Eddie James
  2020-06-02  2:05 ` Joel Stanley
  0 siblings, 1 reply; 3+ messages in thread
From: Eddie James @ 2020-05-20 18:17 UTC (permalink / raw)
  To: openbmc; +Cc: joel, Eddie James

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(-)

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

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH linux dev-5.4] fsi: core: Set slave local bus ownership during init
  2020-05-20 18:17 [PATCH linux dev-5.4] fsi: core: Set slave local bus ownership during init Eddie James
@ 2020-06-02  2:05 ` Joel Stanley
  2020-06-08 15:02   ` Eddie James
  0 siblings, 1 reply; 3+ messages in thread
From: Joel Stanley @ 2020-06-02  2:05 UTC (permalink / raw)
  To: Eddie James, Jeremy Kerr, Andrew Jeffery, Alistair Popple
  Cc: OpenBMC Maillist

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
>

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH linux dev-5.4] fsi: core: Set slave local bus ownership during init
  2020-06-02  2:05 ` Joel Stanley
@ 2020-06-08 15:02   ` Eddie James
  0 siblings, 0 replies; 3+ messages in thread
From: Eddie James @ 2020-06-08 15:02 UTC (permalink / raw)
  To: Joel Stanley, Jeremy Kerr, Andrew Jeffery, Alistair Popple
  Cc: OpenBMC Maillist


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
>>

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2020-06-08 15:02 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-20 18:17 [PATCH linux dev-5.4] fsi: core: Set slave local bus ownership during init Eddie James
2020-06-02  2:05 ` Joel Stanley
2020-06-08 15:02   ` Eddie James

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.