mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "Pali Rohár" <pali.rohar@gmail.com>
To: Andy Shevchenko <andy.shevchenko@gmail.com>
Cc: Mario Limonciello <Mario.Limonciello@dell.com>,
	Darren Hart <dvhart@infradead.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Platform Driver <platform-driver-x86@vger.kernel.org>
Subject: Re: [PATCH] platform/x86: dell-laptop: Allocate buffer on heap rather than globally
Date: Wed, 31 Jan 2018 18:06:39 +0100	[thread overview]
Message-ID: <20180131170639.fhmodmamu4mi6ubw@pali> (raw)
In-Reply-To: <CAHp75VcqYgDCTPqjn9F1SpAbamgPk8B7s4zYAb8csD8oFpLihw@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 1720 bytes --]

On Wednesday 31 January 2018 18:53:19 Andy Shevchenko wrote:
> On Wed, Jan 31, 2018 at 6:46 PM,  <Mario.Limonciello@dell.com> wrote:
> 
> >> > for allocation: ..._alloc_request()
> >> > for filling: _fill_request() / _prepare_request()
> >> >
> >> > or alike.
> >> >
> >> > _set_arguments() not good enough to me.
> >>
> >> Ok. Then we need to stick with 5 arguments... What about name
> >> dell_fill_request()? E.g.
> >>
> >>   struct calling_interface_buffer buffer;
> >>   dell_fill_request(&buffer, 0x2, 0, 0, 0);
> >>   ret = dell_send_request(&buffer, CLASS_INFO, SELECT_RFKILL);
> >>
> >
> > The other alternative is to just define the input of the structure immediately with
> > an initializer, no multi argument filler function.  Like this:
> 
> Either would work for me, though one comment below.
> 
> > -       struct calling_interface_buffer buffer;
> 
> 
> > +       struct calling_interface_buffer buffer = {CLASS_INFO,
> > +                                                 SELECT_RFKILL,
> > +                                                 {0, 0, 0, 0},
> > +                                                 {0, 0, 0, 0}};
> 
> Looking to this approach I would rather provide a macro then.
> 
> #define FILL_REQUEST(a,b,c,d,...) \ // variant FILL_RFKILL_REQUEST(a,b,c,d)
> (struct calling_interface_buffer) { \
>  ... \
> }
> 
> But then it is macro(s) vs. function(s) debate.

Does not matter, I'm fine with either macro or function.

> 
> > -       dell_set_arguments(&buffer, 0, 0, 0, 0);
> > -       ret = dell_send_request(&buffer, CLASS_INFO, SELECT_RFKILL);
> > +       ret = dell_send_request(&buffer);
> 

-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

  reply	other threads:[~2018-01-31 17:06 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-01-30 16:59 Mario Limonciello
2018-01-30 17:17 ` Pali Rohár
2018-01-30 18:14   ` Andy Shevchenko
2018-01-31  9:07     ` Pali Rohár
2018-01-31 16:46       ` Mario.Limonciello
2018-01-31 16:53         ` Andy Shevchenko
2018-01-31 17:06           ` Pali Rohár [this message]
2018-01-31 17:15             ` Andy Shevchenko
2018-01-30 17:20 ` Pali Rohár

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180131170639.fhmodmamu4mi6ubw@pali \
    --to=pali.rohar@gmail.com \
    --cc=Mario.Limonciello@dell.com \
    --cc=andy.shevchenko@gmail.com \
    --cc=dvhart@infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=platform-driver-x86@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®