Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1660191

Re: [RFC PATCH 6/8] firmware: arm_scmi: add initial support for power protocol

Show all headers | View raw

On Wed, Jun 7, 2017 at 6:10 PM, Sudeep Holla <sudeep.holla@arm.com> wrote:

> +struct scmi_msg_resp_power_attributes {
> +       __le16 num_domains;
> +       __le16 reserved;
> +       __le32 stats_addr_low;
> +       __le32 stats_addr_high;
> +       __le32 stats_size;
> +} __packed;
> +
> +struct scmi_msg_resp_power_domain_attributes {
> +       __le32 flags;
> +#define SUPPORTS_STATE_SET_NOTIFY(x)   ((x) & BIT(31))
> +#define SUPPORTS_STATE_SET_ASYNC(x)    ((x) & BIT(30))
> +#define SUPPORTS_STATE_SET_SYNC(x)     ((x) & BIT(29))
> +           u8 name[SCMI_MAX_STR_SIZE];
> +} __packed;

I think it would be better to leave out the __packed here, which
can lead to rather inefficient code. It's only really a problem when
building with -mstrict-align, but it's better to write code in a way that
doesn't rely on that.

> +static int
> +scmi_power_domain_attributes_get(struct scmi_handle *handle, u32 domain,
> +                                struct power_dom_info *dom_info)
> +{
> +       int ret;
> +       struct scmi_xfer *t;
> +       struct scmi_msg_resp_power_domain_attributes *attr;
> +
> +       ret = scmi_one_xfer_init(handle, POWER_DOMAIN_ATTRIBUTES,
> +                                SCMI_PROTOCOL_POWER, sizeof(domain),
> +                                sizeof(*attr), &t);
> +       if (ret)
> +               return ret;
> +
> +       *(__le32 *)t->tx.buf = cpu_to_le32(domain);
> +       attr = (struct scmi_msg_resp_power_domain_attributes *)t->rx.buf;

It seems you require a similar pattern in each caller of scmi_one_xfer_init(),
but it seems a little clumsy to always require those casts, so maybe there
is a nicer way to do this. How about making scmi_one_xfer_init() act
as an allocation function and having it return the buffer or a PTR_ERR?

It also seems odd to have it named 'init' but actually allocate the scmi_xfer
structure rather than filling a local variable that gets passed by reference.

       Arnd

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[RFC PATCH 0/8] firmware: ARM System Control and Management Interface(SCMI) support Sudeep Holla <sudeep.holla@arm.com> - 2017-06-07 18:20 +0200
  [RFC PATCH 8/8] firmware: arm_scmi: probe and initialise all the supported protocols Sudeep Holla <sudeep.holla@arm.com> - 2017-06-07 18:20 +0200
  [RFC PATCH 2/8] firmware: arm_scmi: add basic driver infrastructure for SCMI Sudeep Holla <sudeep.holla@arm.com> - 2017-06-07 18:20 +0200
    Re: [RFC PATCH 2/8] firmware: arm_scmi: add basic driver  infrastructure for SCMI Roy Franz <rfranz@cavium.com> - 2017-06-07 21:20 +0200
      Re: [RFC PATCH 2/8] firmware: arm_scmi: add basic driver  infrastructure for SCMI Sudeep Holla <sudeep.holla@arm.com> - 2017-06-08 11:30 +0200
  [RFC PATCH 7/8] firmware: arm_scmi: add initial support for sensor protocol Sudeep Holla <sudeep.holla@arm.com> - 2017-06-07 18:20 +0200
    Re: [RFC PATCH 7/8] firmware: arm_scmi: add initial support for  sensor protocol Roy Franz <rfranz@cavium.com> - 2017-06-07 21:30 +0200
  [RFC PATCH 3/8] firmware: arm_scmi: add common infrastructure and support for base protocol Sudeep Holla <sudeep.holla@arm.com> - 2017-06-07 18:20 +0200
    Re: [RFC PATCH 3/8] firmware: arm_scmi: add common infrastructure and  support for base protocol Roy Franz <rfranz@cavium.com> - 2017-06-07 21:30 +0200
  [RFC PATCH 6/8] firmware: arm_scmi: add initial support for power protocol Sudeep Holla <sudeep.holla@arm.com> - 2017-06-07 18:20 +0200
    Re: [RFC PATCH 6/8] firmware: arm_scmi: add initial support for power protocol Arnd Bergmann <arnd@arndb.de> - 2017-06-07 22:40 +0200
      Re: [RFC PATCH 6/8] firmware: arm_scmi: add initial support for power  protocol Sudeep Holla <sudeep.holla@arm.com> - 2017-06-08 11:40 +0200
        Re: [RFC PATCH 6/8] firmware: arm_scmi: add initial support for power protocol Arnd Bergmann <arnd@arndb.de> - 2017-06-08 13:10 +0200
          Re: [RFC PATCH 6/8] firmware: arm_scmi: add initial support for power  protocol Sudeep Holla <sudeep.holla@arm.com> - 2017-06-08 13:20 +0200

csiph-web