On 1/29/24 10:50 PM, Siddharth Vadapalli wrote:
Hello Andrew,
On 29/01/24 20:49, Andrew Davis wrote:
On 1/29/24 4:49 AM, Siddharth Vadapalli wrote:
...
int ret;
- syscon = syscon_regmap_lookup_by_phandle(node, "ti,syscon-pcie-ctrl");
+ scm_conf = of_parse_phandle(node, "ti,syscon-pcie-ctrl", 0);
+ if (!scm_conf) {
+ dev_err(dev, "unable to get System Controller node\n");
+ return -ENODEV;
+ }
+
+ syscon = device_node_to_regmap(scm_conf);
Turning the entire "simple-bus" region into a regmap using this
function is just as broken as having it as a "syscon". The core
problem we are solving by getting rid of the blanket syscon nodes
is that it causes multiple mappings of the same register. This
can cause issues with regmap caching, read–modify–write, etc..
What you want to do is add a subnode to the simple-bus, have that
encapsulate just the registers used for PCIe, and have the PCIe
node point to that. Then this patch isn't needed.
For an example, see how it's done for DSS[0].
Thank you for reviewing the patch. I will implement it similar to what's done
for DSS as you pointed out. However, what about the existing SoCs which make use
of the "ti,syscon-pcie-ctrl" property? Do you suggest that I add another
device-tree property for pointing to the PCIE_CTRL register within the CTRL_MMR
region, or do you suggest that I reuse the existing "ti,syscon-pcie-ctrl"
property differently in the SoCs like J784S4 where the scm_conf node is a
"simple-bus"?
The "ti,syscon-pcie-ctrl" property as defined in the device-tree bindings has
two elements with the first being the phandle to the scm_conf node and the
second being the offset of the PCIE_CTRL register. The newer implementation you
are suggesting will either require a new property which accepts only one element
namely the phandle to the node within scm_conf corresponding to the PCIE_CTRL
register. Will adding a new property be acceptable?
Why would you need a new property? If there is no offset to the PCIE_CTRL register
in the new syscon space then just set the offset = 0x0, easy.
Andrew
