Re: [PATCH 2/7] mfd: cros_ec: spi: Add mutex to cros_ec_spi

[Simon Glass]

Hi Doug,

On 18 April 2014 15:15, Doug Anderson  wrote:
>
> Simon,
>
> On Fri, Apr 18, 2014 at 10:28 AM, Simon Glass  wrote:
> > Hi Doug,
> >
> > On 17 April 2014 11:59, Doug Anderson  wrote:
> >> The main transfer function for cros_ec_spi can be called by more than
> >> one client at a time.  Make sure that those clients don't stomp on
> >> each other by locking the bus for the duration of the transfer
> >> function.
> >
> > Is there no lock at the cros_ec layer?
>
> Not with what's upstream.
>
> Locally in the Chromium OS tree:
>
> * You can see that Bill removed the dev_lock at
>  since it wasn't
> used.
>
> * Andrew moved to a common locking scheme later in
>  (thus adding
> roughly the same lock back and using it), but in order to do that
> we've got a dozen pathces in between, some of which are big
> reorganizations.  This includes at least:
>
> 6820b91 CHROMIUM: cros_ec: Tweak struct cros_ec_device for clarity
> 5e8e562 CHROMIUM: Use struct cros_ec_command to communicate with the EC
> 9e7db82 CHROMIUM: cleanup: remove unused fields from struct cros_ec_device
> 866e62d CHROMIUM: cleanup: Remove EC wrapper functions.
> 8a372b2 cros_ec: move locking into cros_ec_cmd_xfer
> 981c4aa cros_ec: stop calling ->cmd_xfer() directly
>
> I think we should take in some of those other changes but I'd rather
> get correctness in first (since people are wanting to use this driver
> in upstream kernel) and get cleanups posted after this lands.  I was
> also trying not to spam the list with a 20-patch series...

That explains it, thank you. I should have guessed that for myself.

Reviewed-by: Simon Glass 

Regards,
Simon
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/7] mfd: cros_ec: spi: Add mutex to cros_ec_spi

[Doug Anderson]

Simon,

On Fri, Apr 18, 2014 at 10:28 AM, Simon Glass  wrote:
> Hi Doug,
>
> On 17 April 2014 11:59, Doug Anderson  wrote:
>> The main transfer function for cros_ec_spi can be called by more than
>> one client at a time.  Make sure that those clients don't stomp on
>> each other by locking the bus for the duration of the transfer
>> function.
>
> Is there no lock at the cros_ec layer?

Not with what's upstream.

Locally in the Chromium OS tree:

* You can see that Bill removed the dev_lock at
 since it wasn't
used.

* Andrew moved to a common locking scheme later in
 (thus adding
roughly the same lock back and using it), but in order to do that
we've got a dozen pathces in between, some of which are big
reorganizations.  This includes at least:

6820b91 CHROMIUM: cros_ec: Tweak struct cros_ec_device for clarity
5e8e562 CHROMIUM: Use struct cros_ec_command to communicate with the EC
9e7db82 CHROMIUM: cleanup: remove unused fields from struct cros_ec_device
866e62d CHROMIUM: cleanup: Remove EC wrapper functions.
8a372b2 cros_ec: move locking into cros_ec_cmd_xfer
981c4aa cros_ec: stop calling ->cmd_xfer() directly

I think we should take in some of those other changes but I'd rather
get correctness in first (since people are wanting to use this driver
in upstream kernel) and get cleanups posted after this lands.  I was
also trying not to spam the list with a 20-patch series...

-Doug
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/7] mfd: cros_ec: spi: Add mutex to cros_ec_spi

[Simon Glass]

Hi Doug,

On 17 April 2014 11:59, Doug Anderson  wrote:
> The main transfer function for cros_ec_spi can be called by more than
> one client at a time.  Make sure that those clients don't stomp on
> each other by locking the bus for the duration of the transfer
> function.

Is there no lock at the cros_ec layer?

Regards,
Simon
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html