On 8/21/19 10:10 AM, Philipp Zabel wrote:
> On Tue, 2019-08-20 at 11:47 -0500, Suman Anna wrote:
>> On 8/20/19 2:37 AM, Tero Kristo wrote:
>>> On 20.8.2019 2.01, Suman Anna wrote:
>>>> Hi Tero,
>>>>
>>>> On 8/19/19 4:32 AM, Tero Kristo wrote:
> [...]
>>>>>>> +{
>>>>>>> + struct omap_reset_data *reset;
>>>>>>> +
>>>>>>> + /*
>>>>>>> + * Check if we have resets. If either rstctl or rstst is
>>>>>>> + * non-zero, we have reset registers in place. Additionally
>>>>>>> + * the flag OMAP_PRM_NO_RSTST implies that we have resets.
>>>>>>> + */
>>>>>>> + if (!prm->data->rstctl && !prm->data->rstst &&
>>>>>>> + !(prm->data->flags & OMAP_PRM_NO_RSTST))
>>>>>>> + return 0;
>>>>>>> +
>>>>>>> + reset = devm_kzalloc(&pdev->dev, sizeof(*reset), GFP_KERNEL);
>>>>>>> + if (!reset)
>>>>>>> + return -ENOMEM;
>>>>>>> +
>>>>>>> + reset->rcdev.owner = THIS_MODULE;
>>>>>>> + reset->rcdev.ops = &omap_reset_ops;
>>>>>>> + reset->rcdev.of_node = pdev->dev.of_node;
>>>>>>> + reset->rcdev.nr_resets = OMAP_MAX_RESETS;
>>>>
>>>> Suggest adding a number of resets to prm->data, and using it so that we
>>>> don't even entertain any resets beyond the actual number of resets.
>>>
>>> Hmm why bother? Accessing a stale reset bit will just cause access to a
>>> reserved bit in the reset register, doing basically nothing. Also, this
>>> would not work for am3/am4 wkup, as there is a single reset bit at an
>>> arbitrary position.
>>
>> The generic convention seems to be defining a reset id value defined
>> from include/dt-bindings/reset/ that can be used to match between the
>> dt-nodes and the reset-controller driver.
>>
>> Philipp,
>> Any comments?
>
> Are there only reset bits and reserved bits in the range accessible by
> [0..OMAP_MAX_RESETS] or are ther bits with another function as well?
Thanks Philipp, these are just reset bits and reserved bits.
> If the latter is the case, I would prefer enumerating the resets in a
> dt-bindings header, with the driver containing an enum -> reg/bit
> position lookup table.
>
> In general, assuming the device tree contains no errors, this should not
> matter much, but I think it is nice if the reset driver, even with a
> misconfigured device tree, can't write into arbitrary bit fields.
Tero,
Can you add a check for this if possible?
regards
Suman
