Re: [PATCH v2 2/3] soc: rockchip: add driver handling grf setup

Shawn Lin
2016-11-17T01:38:11Z

Hi Heiko,

在 2016/11/16 17:58, Heiko Stuebner 写道:
> Hi Shawn,
>
> Am Mittwoch, 16. November 2016, 17:33:21 CET schrieb Shawn Lin:
>> 在 2016/11/16 6:38, Heiko Stuebner 写道:
>>> The General Register Files are an area of registers containing a lot
>>> of single-bit settings for numerous components as well full components
>>> like usbphy control. Therefore all used components are accessed
>>> via the syscon provided by the grf nodes or from the sub-devices
>>> created through the simple-mfd created from the grf node.
>>>

----8<----------------
[...]

>>> + for (i = 0; i < grf_info->num_values; i++) {
>>> + const struct rockchip_grf_value *val = &grf_info->values[i];
>>> +
>>> + pr_debug("%s: adjusting %s in %#6x to %#10x\n", __func__,
>>> + val->desc, val->reg, val->val);
>>> + ret = regmap_write(grf, val->reg, val->val);
>>> + if (ret < 0)
>>> + pr_err("%s: write to %#6x failed with %d\n",
>>> + __func__, val->reg, ret);
>>
>> So, when failing to do one of the settings, should we still let it goes?
>> Sometimes the log of postcore_initcall is easy to be neglected when
>> people finally find problems later but the very earlier log was missing
>> due to whatever reason like buffer limitation, etc.
>
> I expect errors here to be very rare. I.e. Doug thought that regmap might
> return errors if we write outside the mapped region, which would mean someone
> really messed up in this core component or a core-element of the dts.
> But in general the GRF is always a mmio-regmap, so there won't be any i2c/spi/
> whatever errors possible.

I was just thinking about the scalability that in the future some of the
GRF settings may depend on genpd or clock even if they are only a
mmio-regmap but they don't belong to the general pd/clock for GRF, for
instance, some of the PHYs' or controller's cap(like emmc) settings.

But, IIRC, you suggested this driver shouldn't be a catchall, and we
now ask the drivers of controllers and phys to take over this, so I
guess we won't put those settings here ever. :)

Thanks for explaining this.

>
> Also settings are pretty individual, so if setting 1 fails, setting 2 can
> still succeed and the boot can continue somewhat farther.
>

sure

> The overall target is supposed to always boot as far as possible, so that
> people might recover or get more information and not fail (and die) early.
>

ok, got it.

>
> Heiko
>
>
>

--
Best Regards
Shawn Lin

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org