From: Romain Naour <romain.naour@smile.fr>
To: Mark Brown <broonie@kernel.org>
Cc: linux-kernel@vger.kernel.org, linux-omap@vger.kernel.org,
tony@atomide.com, lgirdwood@gmail.com,
Romain Naour <romain.naour@skf.com>
Subject: Re: [PATCH] regulator: ti-abb: don't use devm_platform_ioremap_resource_byname for shared interrupt register
Date: Tue, 23 Jan 2024 10:48:50 +0100 [thread overview]
Message-ID: <b5decdec-f33a-4997-a607-e31290fcd83c@smile.fr> (raw)
In-Reply-To: <1b2b5afc-308f-48bb-924a-2c29371abfc9@sirena.org.uk>
Hello,
Le 22/01/2024 à 18:30, Mark Brown a écrit :
> On Mon, Jan 22, 2024 at 06:04:42PM +0100, Romain Naour wrote:
>
>> We can't use devm_platform_ioremap_resource_byname() to remap the
>> interrupt register that can be shared between
>> regulator-abb-{ivahd,dspeve,gpu} drivers instance.
>
> ...
>
>> The commit b36c6b1887ff (regulator: ti-abb: Make use of the helper
>> function devm_ioremap related) overlooked the following comment
>> explaining why devm_ioremap() is used in this case:
>
>> /*
>> * We may have shared interrupt register offsets which are
>> * write-1-to-clear between domains ensuring exclusivity.
>> */
>
> I have to say that I wouldn't infer from that comment that there is any
> reason why _byname() won't work - one would generally expect that a
> get_resource_by_name() followed by an ioremap() of that resource would
> be equivalent to the combined helper. Based on the commit log here I
> frankly have no idea what the issue is. You should also add something
> to the code which makes it clear what the issue is so the same
> conversion isn't performed again, assuming that the fix isn't in the
> helper.
I'm agree with you about the existing comment that is not really crystal clear.
The combined helper introduce a call to devm_request_mem_region() that create a
new busy resource region on PRM_IRQSTATUS_MPU register (0x4ae06010). The first
devm_request_mem_region() call succeed for regulator-abb-ivahd but fail for the
two other regulator-abb-dspeve and regulator-abb-gpu.
Here is the iomem content without this patch:
# cat /proc/iomem | grep -i 4ae06
4ae06010-4ae06013 : 4ae07e34.regulator-abb-ivahd int-address
4ae06014-4ae06017 : 4ae07ddc.regulator-abb-mpu int-address
regulator-abb-dspeve and regulator-abb-gpu are missing due to
devm_request_mem_region() failure (EBUSY)
I don't know how to fix this issue keeping
devm_platform_ioremap_resource_byname() when the same address is used several
time... suggestion welcome.
>
>>
>> Fixes:
>
> You're missing the commit here.
>
>> This partially reverts commit b36c6b1887ffc6b58b556120bfbd511880515247.
>
> Please include human readable descriptions of things like commits and
> issues being discussed in e-mail in your mails, this makes them much
> easier for humans to read especially when they have no internet access.
> I do frequently catch up on my mail on flights or while otherwise
> travelling so this is even more pressing for me than just being about
> making things a bit easier to read.
I added such human description above in the commit log but forgot to update this
one, sorry.
Thank you for the review.
Best regards,
Romain
prev parent reply other threads:[~2024-01-23 9:48 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-01-22 17:04 [PATCH] regulator: ti-abb: don't use devm_platform_ioremap_resource_byname for shared interrupt register Romain Naour
2024-01-22 17:30 ` Mark Brown
2024-01-23 9:48 ` Romain Naour [this message]
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=b5decdec-f33a-4997-a607-e31290fcd83c@smile.fr \
--to=romain.naour@smile.fr \
--cc=broonie@kernel.org \
--cc=lgirdwood@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-omap@vger.kernel.org \
--cc=romain.naour@skf.com \
--cc=tony@atomide.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).