v5 -> v6:
- Resending due to no responses.
- Change of base to next-20231124 (no changes to the patch)
Link to v5: https://lore.kernel.org/r/[email protected]
v4 -> v5:
- Pick up tags
- Rebase on Rob's of_ header untanglement
Link to v4: https://lore.kernel.org/r/[email protected]
v3 -> v4:
- Fix up indentation in the bindings patch
- Add an example glink-edge subnode to remoteproc-rpm (its bindings
require that..)
Link to v3: https://lore.kernel.org/r/[email protected]
v2 -> v3:
- Fix the example
- Pick up tags
- remove the outdated example from the cover letter, check bindings
should you want to see one
The bindings for the wrapper node used in the yaml example are merged
in qcom/for-next
Link to v2: https://lore.kernel.org/r/[email protected]
v1 -> v2:
- deprecate 'reg', make qcom,rpm-msg-ram required [1/2]
- Use devm_ioremap() [2/2]
Link to v1: https://lore.kernel.org/r/[email protected]
Depends on resolution of https://github.com/devicetree-org/dt-schema/issues/104
The MPM (and some other things, irrelevant to this patchset) resides
(as far as the ARM cores are concerned, anyway) in a MMIO-mapped region
that's a portion of the RPM (low-power management core)'s RAM, known
as the RPM Message RAM. Representing this relation in the Device Tree
creates some challenges, as one would either have to treat a memory
region as a bus, map nodes in a way such that their reg-s would be
overlapping, or supply the nodes with a slice of that region.
This series implements the third option, by adding a qcom,rpm-msg-ram
property, which has been used for some drivers poking into this region
before. Bindings ABI compatibility is preserved through keeping the
"normal" (a.k.a read the reg property and map that region) way of
passing the register space.
Signed-off-by: Konrad Dybcio <[email protected]>
---
Konrad Dybcio (2):
dt-bindings: interrupt-controller: mpm: Pass MSG RAM slice through phandle
irqchip: irq-qcom-mpm: Support passing a slice of SRAM as reg space
.../bindings/interrupt-controller/qcom,mpm.yaml | 52 +++++++++++++++-------
drivers/irqchip/irq-qcom-mpm.c | 21 +++++++--
2 files changed, 53 insertions(+), 20 deletions(-)
---
base-commit: 8c9660f6515396aba78d1168d2e17951d653ebf2
change-id: 20230328-topic-msgram_mpm-c688be3bc294
Best regards,
--
Konrad Dybcio <[email protected]>
The MPM hardware is accessible to us from the ARM CPUs through a shared
memory region (RPM MSG RAM) that's also concurrently accessed by other
kinds of cores on the system (like modem, ADSP etc.). Modeling this
relation in a (somewhat) sane manner in the device tree basically
requires us to either present the MPM as a child of said memory region
(which makes little sense, as a mapped memory carveout is not a bus),
define nodes which bleed their register spaces into one another, or
passing their slice of the MSG RAM through some kind of a property.
Go with the third option and add a way to map a region passed through
the "qcom,rpm-msg-ram" property as our register space.
The current way of using 'reg' is preserved for ABI reasons.
Acked-by: Shawn Guo <[email protected]>
Signed-off-by: Konrad Dybcio <[email protected]>
---
drivers/irqchip/irq-qcom-mpm.c | 21 ++++++++++++++++++---
1 file changed, 18 insertions(+), 3 deletions(-)
diff --git a/drivers/irqchip/irq-qcom-mpm.c b/drivers/irqchip/irq-qcom-mpm.c
index 7124565234a5..7115e3056aa5 100644
--- a/drivers/irqchip/irq-qcom-mpm.c
+++ b/drivers/irqchip/irq-qcom-mpm.c
@@ -14,6 +14,7 @@
#include <linux/mailbox_client.h>
#include <linux/module.h>
#include <linux/of.h>
+#include <linux/of_address.h>
#include <linux/of_platform.h>
#include <linux/platform_device.h>
#include <linux/pm_domain.h>
@@ -322,8 +323,10 @@ static int qcom_mpm_init(struct device_node *np, struct device_node *parent)
struct device *dev = &pdev->dev;
struct irq_domain *parent_domain;
struct generic_pm_domain *genpd;
+ struct device_node *msgram_np;
struct qcom_mpm_priv *priv;
unsigned int pin_cnt;
+ struct resource res;
int i, irq;
int ret;
@@ -374,9 +377,21 @@ static int qcom_mpm_init(struct device_node *np, struct device_node *parent)
raw_spin_lock_init(&priv->lock);
- priv->base = devm_platform_ioremap_resource(pdev, 0);
- if (IS_ERR(priv->base))
- return PTR_ERR(priv->base);
+ /* If we have a handle to an RPM message ram partition, use it. */
+ msgram_np = of_parse_phandle(np, "qcom,rpm-msg-ram", 0);
+ if (msgram_np) {
+ ret = of_address_to_resource(msgram_np, 0, &res);
+ /* Don't use devm_ioremap_resource, as we're accessing a shared region. */
+ priv->base = devm_ioremap(dev, res.start, resource_size(&res));
+ of_node_put(msgram_np);
+ if (IS_ERR(priv->base))
+ return PTR_ERR(priv->base);
+ } else {
+ /* Otherwise, fall back to simple MMIO. */
+ priv->base = devm_platform_ioremap_resource(pdev, 0);
+ if (IS_ERR(priv->base))
+ return PTR_ERR(priv->base);
+ }
for (i = 0; i < priv->reg_stride; i++) {
qcom_mpm_write(priv, MPM_REG_ENABLE, i, 0);
--
2.43.0
On 25/11/2023 14:27, Konrad Dybcio wrote:
> The MPM hardware is accessible to us from the ARM CPUs through a shared
> memory region (RPM MSG RAM) that's also concurrently accessed by other
> kinds of cores on the system (like modem, ADSP etc.). Modeling this
> relation in a (somewhat) sane manner in the device tree basically
> requires us to either present the MPM as a child of said memory region
> (which makes little sense, as a mapped memory carveout is not a bus),
> define nodes which bleed their register spaces into one another, or
> passing their slice of the MSG RAM through some kind of a property.
>
> Go with the third option and add a way to map a region passed through
> the "qcom,rpm-msg-ram" property as our register space.
>
> The current way of using 'reg' is preserved for ABI reasons.
>
> Acked-by: Shawn Guo <[email protected]>
> Signed-off-by: Konrad Dybcio <[email protected]>
> ---
> drivers/irqchip/irq-qcom-mpm.c | 21 ++++++++++++++++++---
> 1 file changed, 18 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/irqchip/irq-qcom-mpm.c b/drivers/irqchip/irq-qcom-mpm.c
> index 7124565234a5..7115e3056aa5 100644
> --- a/drivers/irqchip/irq-qcom-mpm.c
> +++ b/drivers/irqchip/irq-qcom-mpm.c
> @@ -14,6 +14,7 @@
> #include <linux/mailbox_client.h>
> #include <linux/module.h>
> #include <linux/of.h>
> +#include <linux/of_address.h>
> #include <linux/of_platform.h>
> #include <linux/platform_device.h>
> #include <linux/pm_domain.h>
> @@ -322,8 +323,10 @@ static int qcom_mpm_init(struct device_node *np, struct device_node *parent)
> struct device *dev = &pdev->dev;
> struct irq_domain *parent_domain;
> struct generic_pm_domain *genpd;
> + struct device_node *msgram_np;
> struct qcom_mpm_priv *priv;
> unsigned int pin_cnt;
> + struct resource res;
> int i, irq;
> int ret;
>
> @@ -374,9 +377,21 @@ static int qcom_mpm_init(struct device_node *np, struct device_node *parent)
>
> raw_spin_lock_init(&priv->lock);
>
> - priv->base = devm_platform_ioremap_resource(pdev, 0);
> - if (IS_ERR(priv->base))
> - return PTR_ERR(priv->base);
> + /* If we have a handle to an RPM message ram partition, use it. */
> + msgram_np = of_parse_phandle(np, "qcom,rpm-msg-ram", 0);
> + if (msgram_np) {
> + ret = of_address_to_resource(msgram_np, 0, &res);
You are capturing the return value but doing nothing with it.
One of
if (ret) {
of_node_put(msgram_np);
return ret;
}
or just drop the ret =
if you are sure of_address_to_resource() can never return an error for
your use-case.
Once fixed.
Reviewed-by: Bryan O'Donoghue <[email protected]>
On 25.11.2023 16:17, Bryan O'Donoghue wrote:
> On 25/11/2023 14:27, Konrad Dybcio wrote:
>> The MPM hardware is accessible to us from the ARM CPUs through a shared
>> memory region (RPM MSG RAM) that's also concurrently accessed by other
>> kinds of cores on the system (like modem, ADSP etc.). Modeling this
>> relation in a (somewhat) sane manner in the device tree basically
>> requires us to either present the MPM as a child of said memory region
>> (which makes little sense, as a mapped memory carveout is not a bus),
>> define nodes which bleed their register spaces into one another, or
>> passing their slice of the MSG RAM through some kind of a property.
>>
>> Go with the third option and add a way to map a region passed through
>> the "qcom,rpm-msg-ram" property as our register space.
>>
>> The current way of using 'reg' is preserved for ABI reasons.
>>
>> Acked-by: Shawn Guo <[email protected]>
>> Signed-off-by: Konrad Dybcio <[email protected]>
>> ---
>> drivers/irqchip/irq-qcom-mpm.c | 21 ++++++++++++++++++---
>> 1 file changed, 18 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/irqchip/irq-qcom-mpm.c b/drivers/irqchip/irq-qcom-mpm.c
>> index 7124565234a5..7115e3056aa5 100644
>> --- a/drivers/irqchip/irq-qcom-mpm.c
>> +++ b/drivers/irqchip/irq-qcom-mpm.c
>> @@ -14,6 +14,7 @@
>> #include <linux/mailbox_client.h>
>> #include <linux/module.h>
>> #include <linux/of.h>
>> +#include <linux/of_address.h>
>> #include <linux/of_platform.h>
>> #include <linux/platform_device.h>
>> #include <linux/pm_domain.h>
>> @@ -322,8 +323,10 @@ static int qcom_mpm_init(struct device_node *np, struct device_node *parent)
>> struct device *dev = &pdev->dev;
>> struct irq_domain *parent_domain;
>> struct generic_pm_domain *genpd;
>> + struct device_node *msgram_np;
>> struct qcom_mpm_priv *priv;
>> unsigned int pin_cnt;
>> + struct resource res;
>> int i, irq;
>> int ret;
>> @@ -374,9 +377,21 @@ static int qcom_mpm_init(struct device_node *np, struct device_node *parent)
>> raw_spin_lock_init(&priv->lock);
>> - priv->base = devm_platform_ioremap_resource(pdev, 0);
>> - if (IS_ERR(priv->base))
>> - return PTR_ERR(priv->base);
>> + /* If we have a handle to an RPM message ram partition, use it. */
>> + msgram_np = of_parse_phandle(np, "qcom,rpm-msg-ram", 0);
>> + if (msgram_np) {
>> + ret = of_address_to_resource(msgram_np, 0, &res);
>
> You are capturing the return value but doing nothing with it.
Oops you're right
>
> One of
>
> if (ret) {
> of_node_put(msgram_np);
> return ret;
> }
>
> or just drop the ret =
>
> if you are sure of_address_to_resource() can never return an error for your use-case.
Never say never!
Konrad