diff mbox

[03/10] mfd: cros_ec: Allow static din/dout buffers with cros_ec_register()

Message ID 1402954800-28215-4-git-send-email-dianders@chromium.org (mailing list archive)
State Not Applicable, archived
Headers show

Commit Message

Doug Anderson June 16, 2014, 9:39 p.m. UTC
From: Bill Richardson <wfrichar@chromium.org>

The lower-level driver may want to provide its own buffers. If so,
there's no need to allocate new ones.  This already happens to work
just fine (since we check for size of 0 and use devm allocation), but
it's good to document it.

[dianders: Resolved conflicts; documented that no code changes needed
on mainline]

Signed-off-by: Bill Richardson <wfrichar@chromium.org>
Signed-off-by: Doug Anderson <dianders@chromium.org>
---
 include/linux/mfd/cros_ec.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Simon Glass June 18, 2014, 3:29 a.m. UTC | #1
On 16 June 2014 14:39, Doug Anderson <dianders@chromium.org> wrote:
> From: Bill Richardson <wfrichar@chromium.org>
>
> The lower-level driver may want to provide its own buffers. If so,
> there's no need to allocate new ones.  This already happens to work
> just fine (since we check for size of 0 and use devm allocation), but
> it's good to document it.
>
> [dianders: Resolved conflicts; documented that no code changes needed
> on mainline]
>
> Signed-off-by: Bill Richardson <wfrichar@chromium.org>
> Signed-off-by: Doug Anderson <dianders@chromium.org>

Reviewed-by: Simon Glass <sjg@chromium.org>
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Lee Jones June 18, 2014, 7:53 a.m. UTC | #2
On Mon, 16 Jun 2014, Doug Anderson wrote:

> From: Bill Richardson <wfrichar@chromium.org>
> 
> The lower-level driver may want to provide its own buffers. If so,
> there's no need to allocate new ones.  This already happens to work
> just fine (since we check for size of 0 and use devm allocation), but
> it's good to document it.
> 
> [dianders: Resolved conflicts; documented that no code changes needed
> on mainline]
> 
> Signed-off-by: Bill Richardson <wfrichar@chromium.org>
> Signed-off-by: Doug Anderson <dianders@chromium.org>
> ---
>  include/linux/mfd/cros_ec.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h
> index 7e9fe6e..2ee3190 100644
> --- a/include/linux/mfd/cros_ec.h
> +++ b/include/linux/mfd/cros_ec.h
> @@ -68,8 +68,8 @@ struct cros_ec_msg {
>   * We use this alignment to keep ARM and x86 happy. Probably word
>   * alignment would be OK, there might be a small performance advantage
>   * to using dword.
> - * @din_size: size of din buffer
> - * @dout_size: size of dout buffer
> + * @din_size: size of din buffer to allocate (zero to use static din)
> + * @dout_size: size of dout buffer to allocate (zero to use static dout)

Why don't these use your new format i.e. doutsize, etc?

>   * @command_send: send a command
>   * @command_recv: receive a command
>   * @ec_name: name of EC device (e.g. 'chromeos-ec')
Doug Anderson June 18, 2014, 4:35 p.m. UTC | #3
Lee,

On Wed, Jun 18, 2014 at 12:53 AM, Lee Jones <lee.jones@linaro.org> wrote:
> On Mon, 16 Jun 2014, Doug Anderson wrote:
>
>> From: Bill Richardson <wfrichar@chromium.org>
>>
>> The lower-level driver may want to provide its own buffers. If so,
>> there's no need to allocate new ones.  This already happens to work
>> just fine (since we check for size of 0 and use devm allocation), but
>> it's good to document it.
>>
>> [dianders: Resolved conflicts; documented that no code changes needed
>> on mainline]
>>
>> Signed-off-by: Bill Richardson <wfrichar@chromium.org>
>> Signed-off-by: Doug Anderson <dianders@chromium.org>
>> ---
>>  include/linux/mfd/cros_ec.h | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h
>> index 7e9fe6e..2ee3190 100644
>> --- a/include/linux/mfd/cros_ec.h
>> +++ b/include/linux/mfd/cros_ec.h
>> @@ -68,8 +68,8 @@ struct cros_ec_msg {
>>   * We use this alignment to keep ARM and x86 happy. Probably word
>>   * alignment would be OK, there might be a small performance advantage
>>   * to using dword.
>> - * @din_size: size of din buffer
>> - * @dout_size: size of dout buffer
>> + * @din_size: size of din buffer to allocate (zero to use static din)
>> + * @dout_size: size of dout buffer to allocate (zero to use static dout)
>
> Why don't these use your new format i.e. doutsize, etc?

Ah, you mean like the new "struct cros_ec_command" that's switched to
in (mfd: cros_ec: Use struct cros_ec_command to communicate with the
EC)?  I don't know--it seems rather arbitrary.  Personally I like
having the underscore (thus if we have to change I'd advocate changing
"struct cros_ec_command").

The inconsistency doesn't bother me terribly and it will be more work
to cherry-pick future patches.  Since it didn't sound like you are
insisting then I won't change this, but if you say that you want me to
change it I'm more than happy to.

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

Patch

diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h
index 7e9fe6e..2ee3190 100644
--- a/include/linux/mfd/cros_ec.h
+++ b/include/linux/mfd/cros_ec.h
@@ -68,8 +68,8 @@  struct cros_ec_msg {
  * We use this alignment to keep ARM and x86 happy. Probably word
  * alignment would be OK, there might be a small performance advantage
  * to using dword.
- * @din_size: size of din buffer
- * @dout_size: size of dout buffer
+ * @din_size: size of din buffer to allocate (zero to use static din)
+ * @dout_size: size of dout buffer to allocate (zero to use static dout)
  * @command_send: send a command
  * @command_recv: receive a command
  * @ec_name: name of EC device (e.g. 'chromeos-ec')