Re: [PATCH 2/8] soc: ti: add initial PRM driver with reset control support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 21.8.2019 18.45, Suman Anna wrote:
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?

Well, I can enforce the usage of reset bit mapping, which I have already implemented for some SoCs like am33xx. If the specific ID is not found, I can bail out. So, basically in this example requesting reset at index 3 would succeed, but it would fail for any other ID; this would be direct HW bit mapping.

-Tero
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux