[PATCH linux dev-5.8] fsi: Aspeed: Add mutex to protect HW access

[Joel Stanley]

On Wed, 30 Sep 2020 at 22:34, Milton Miller II <miltonm at us.ibm.com> wrote:
>
> On September 29, 2020 around 1:50PM in some timezone, Eddie James wrote:
> >
> >There is nothing to prevent multiple commands being executed
> >simultaneously. Add a mutex to prevent this.
> >
> >Signed-off-by: Eddie James <eajames at linux.ibm.com>

> >@@ -597,6 +608,7 @@ static int fsi_master_aspeed_probe(struct
> >platform_device *pdev)
> >
> >       dev_set_drvdata(&pdev->dev, aspeed);
> >
> >+      mutex_init(&aspeed->lock);
> >       aspeed_master_init(aspeed);
>
> So you initialize the lock here in the probe function before its
> used, good.  I notice its not taken in aspeed_master_init nor over
> the opb_read for the version register.  Both are called from the
> probe function and presumably are therefore the sole context
> available, but having it taken could be considered a good for
> consistency.
>
> Are there any concerns that this is part of the fsi master
> context if we wanted to use both fsi buses in the future?

If we use the other FSI master, then it would have a second instance
of the driver so we would be fine.

If/when we add support for the second OPB bus the driver will need a
overhaul, so reworking the locking will form part of that work.

> Reviewed-by: Milton Miller <miltonm at us.ibm.com>

Reviewed-by: Joel Stanley <joel at jms.id.au>
Fixes: 606397d67f41 ("fsi: Add ast2600 master driver")

I have merged this to the openbmc tree.

Eddie, I know this was written to fix the sbe fifo usage. Did you have
a (simplified) test workload that showed the bug that you could share?

Please send this to the upstream lists with the r-b and fixes tags.

Cheers,

Joel