Re: [PATCH 08/17] clk: imx: convert to devm_platform_ioremap_resource

["Andreas Färber" <afaerber@suse.de>]

Am 11.12.19 um 18:57 schrieb Paul Walmsley:
> On Tue, 10 Dec 2019, Andreas Färber wrote:
> 
>> I have similar cases with Realtek where registers are simply not grouped
>> into convenient blocks but spread across large memory regions.
> 
> At the hardware level, registers are grouped into IP blocks, to simply 
> both design integration and address decoding.

Reality shows, not all vendors/chips always care about simplification.

Blocks do have names, but they don't always group registers of the same
kind, as Linux expects it - be it for historical backwards compatibility
reasons or because Linux/Android wasn't their main use case in the past.
Firmware developers won't care where their registers are located.

>  Not knowing which Realtek 
> device you're referring to,

Arm based RTD1195 and RTD1295/RTD1395/RTD1619/RTD1319 SoC families,
which I maintain.

> most likely it's the same situation as with 
> the IMX8M TRM, where the DT data doesn't match the underlying reality of 
> the hardware.  In those cases the best approach is usually to just fix the 
> DT data.

No, you're not reading me. My DT data matches the hardware as far as I
know it. You can be really happy that you can login to get NXP manuals;
for other vendors, manuals simply don't exist and we have to deduce DT
from register names/offsets ourselves. Reality is messy!

Just please accept that hardware does not always allow for unique
contiguous memory reservations, and we therefore cannot force these
types of reservations onto everybody.

There might be an opportunity for a new helper with even longer name
that does the expected combination of actions. But is it worth it?
People seem to have stopped giving motivations for their patches in
commit message or cover letter, so it remains entirely unclear how else
one might satisfy the submitter's goals while keeping your code working.
(Also referring to unjustified style-only cleanups popping up lately.)
"to simplify code" is not much to go on, it sounds like a style cleanup
without any practical error avoidance benefits nor an API to be dropped.

Note that I did not receive any cover letter accompanying this patch,
but was CC'ed on plenty of other patches like this one that I'm not
maintainer of, leading me to assume that none was sent.

Alternatively one could do the reservations decoupled from DT inside the
driver, but again not using this suggested helper.

From what I read on other such patches, apparently some Coccinelle build
target emits warnings when it matches some pattern for potential
refactoring, which people then set out to resolve, without understanding
the code they touch or being able to actually test it. That's probably
the root cause that someone would need to tackle - whitelisting
fully-intentional usages of certain APIs to protect against unwarranted
refactorings, or otherwise making sure that people don't get inspired to
in their best intentions break other people's code. I assume kbuild bot
doesn't send out such cocci warnings to us maintainers for good reasons.

A completely fragmented DT with either dozens of reg entries for single
registers or distinct compatible strings for individual registers, to
give them their own DT nodes, is not really handy, compared to one or
two larger clk nodes that handle reg offsets under the hood, without
impacting public DT bindings (e.g., bumping reg's maxItems, clk header).

If you care about modeling this, you're welcome to participate in patch
review @ DTML/LAKML/LRSML. So far there's largely been a yawning silence
in response to my patches introducing syscon and simple-mfd as cleanups,
before things get worse as we add to the DT. Following an unreviewed clk
RFC of mine two years back, there's now been a clk patchset from Realtek
that got a load of review comments from me, waiting for a v2.

If you don't care, then please don't lecture us about how you think
other people's hardware should ideally be like. That's not helpful.

Regards,
Andreas

-- 
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer
HRB 36809 (AG Nürnberg)

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek