All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Rafał Miłecki" <rafal@milecki.pl>
To: "Srinivas Kandagatla" <srinivas.kandagatla@linaro.org>,
	"Rafał Miłecki" <zajec5@gmail.com>,
	"Rob Herring" <robh+dt@kernel.org>
Cc: devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-mips@vger.kernel.org,
	Florian Fainelli <f.fainelli@gmail.com>,
	Vivek Unune <npcomplete13@gmail.com>,
	bcm-kernel-feedback-list@broadcom.com,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/2] nvmem: iomap: new driver exposing NVMEM accessible using I/O mapping
Date: Fri, 5 Mar 2021 11:39:36 +0100	[thread overview]
Message-ID: <ba4e4376-7d75-4b0e-9d59-26e32dbe2029@milecki.pl> (raw)
In-Reply-To: <35e498b6-3b2c-d154-db00-d755af339b60@linaro.org>

On 05.03.2021 11:33, Srinivas Kandagatla wrote:
> On 05/03/2021 10:24, Rafał Miłecki wrote:
>>>>
>>>> +static int iomap_read(void *context, unsigned int offset, void *val,
>>>> +              size_t bytes)
>>>> +{
>>>> +    struct iomap *priv = context;
>>>> +    u8 *src = priv->base + offset;
>>>> +    u8 *dst = val;
>>>> +    size_t tmp;
>>>> +
>>>> +    tmp = offset % 4;
>>>> +    memcpy_fromio(dst, src, tmp);
>>>> +    dst += tmp;
>>>> +    src += tmp;
>>>> +    bytes -= tmp;
>>>> +
>>>> +    tmp = rounddown(bytes, 4);
>>>> +    __ioread32_copy(dst, src, tmp / 4);
>>>> +    dst += tmp;
>>>> +    src += tmp;
>>>> +    bytes -= tmp;
>>>> +
>>>> +    memcpy_fromio(dst, src, bytes);
>>>> +
>>>
>>>
>>> You could just do this!
>>>
>>>      while (bytes--)
>>>          *val++ = readb(priv->base + offset + i++);
>>
>> Do you mean that as replacement for "memcpy_fromio" or the whole
>> function code?
> 
> Yes please!
> 
>> The reason for using __ioread32_copy() was to improve reading
>> performance (using aligned 32 bit access where possible). I'm not sure
>> if that really matters?
> 
> Just simple while loop is much readable than the previous code TBH!
> 
>>
> 
>> P.S.
>> Please don't yell at me in every sentence :( Makes me a bit sad :(
> Sorry!! I did not mean anything as such! :-)

All clear (I hope)! Thanks for your review!

WARNING: multiple messages have this Message-ID (diff)
From: "Rafał Miłecki" <rafal@milecki.pl>
To: "Srinivas Kandagatla" <srinivas.kandagatla@linaro.org>,
	"Rafał Miłecki" <zajec5@gmail.com>,
	"Rob Herring" <robh+dt@kernel.org>
Cc: devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-mips@vger.kernel.org,
	Florian Fainelli <f.fainelli@gmail.com>,
	Vivek Unune <npcomplete13@gmail.com>,
	bcm-kernel-feedback-list@broadcom.com,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/2] nvmem: iomap: new driver exposing NVMEM accessible using I/O mapping
Date: Fri, 5 Mar 2021 11:39:36 +0100	[thread overview]
Message-ID: <ba4e4376-7d75-4b0e-9d59-26e32dbe2029@milecki.pl> (raw)
In-Reply-To: <35e498b6-3b2c-d154-db00-d755af339b60@linaro.org>

On 05.03.2021 11:33, Srinivas Kandagatla wrote:
> On 05/03/2021 10:24, Rafał Miłecki wrote:
>>>>
>>>> +static int iomap_read(void *context, unsigned int offset, void *val,
>>>> +              size_t bytes)
>>>> +{
>>>> +    struct iomap *priv = context;
>>>> +    u8 *src = priv->base + offset;
>>>> +    u8 *dst = val;
>>>> +    size_t tmp;
>>>> +
>>>> +    tmp = offset % 4;
>>>> +    memcpy_fromio(dst, src, tmp);
>>>> +    dst += tmp;
>>>> +    src += tmp;
>>>> +    bytes -= tmp;
>>>> +
>>>> +    tmp = rounddown(bytes, 4);
>>>> +    __ioread32_copy(dst, src, tmp / 4);
>>>> +    dst += tmp;
>>>> +    src += tmp;
>>>> +    bytes -= tmp;
>>>> +
>>>> +    memcpy_fromio(dst, src, bytes);
>>>> +
>>>
>>>
>>> You could just do this!
>>>
>>>      while (bytes--)
>>>          *val++ = readb(priv->base + offset + i++);
>>
>> Do you mean that as replacement for "memcpy_fromio" or the whole
>> function code?
> 
> Yes please!
> 
>> The reason for using __ioread32_copy() was to improve reading
>> performance (using aligned 32 bit access where possible). I'm not sure
>> if that really matters?
> 
> Just simple while loop is much readable than the previous code TBH!
> 
>>
> 
>> P.S.
>> Please don't yell at me in every sentence :( Makes me a bit sad :(
> Sorry!! I did not mean anything as such! :-)

All clear (I hope)! Thanks for your review!

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2021-03-05 10:59 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-04 14:41 [PATCH 1/2] dt-bindings: nvmem: add binding for I/O mapped NVMEM Rafał Miłecki
2021-03-04 14:41 ` Rafał Miłecki
2021-03-04 14:41 ` [PATCH 2/2] nvmem: iomap: new driver exposing NVMEM accessible using I/O mapping Rafał Miłecki
2021-03-04 14:41   ` Rafał Miłecki
2021-03-05 10:02   ` Srinivas Kandagatla
2021-03-05 10:02     ` Srinivas Kandagatla
2021-03-05 10:24     ` Rafał Miłecki
2021-03-05 10:24       ` Rafał Miłecki
2021-03-05 10:33       ` Srinivas Kandagatla
2021-03-05 10:33         ` Srinivas Kandagatla
2021-03-05 10:39         ` Rafał Miłecki [this message]
2021-03-05 10:39           ` Rafał Miłecki

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=ba4e4376-7d75-4b0e-9d59-26e32dbe2029@milecki.pl \
    --to=rafal@milecki.pl \
    --cc=bcm-kernel-feedback-list@broadcom.com \
    --cc=devicetree@vger.kernel.org \
    --cc=f.fainelli@gmail.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mips@vger.kernel.org \
    --cc=npcomplete13@gmail.com \
    --cc=robh+dt@kernel.org \
    --cc=srinivas.kandagatla@linaro.org \
    --cc=zajec5@gmail.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.