diff mbox series

[v3,2/2] memory: tegra: make sid and broadcast regions optional

Message ID 20240412130540.28447-3-sumitg@nvidia.com
State New
Headers show
Series memory: tegra: Skip restricted register access from Guest | expand

Commit Message

Sumit Gupta April 12, 2024, 1:05 p.m. UTC
MC SID and Broadbast channel register access is restricted for Guest VM.
In Tegra MC driver, consider both the regions as optional and skip
access to restricted registers from Guest if a region is not present
in Guest DT.

Signed-off-by: Sumit Gupta <sumitg@nvidia.com>
---
 drivers/memory/tegra/mc.c       |  9 ++++++++-
 drivers/memory/tegra/mc.h       | 22 ++++++++++++----------
 drivers/memory/tegra/tegra186.c | 25 +++++++++++++------------
 3 files changed, 33 insertions(+), 23 deletions(-)

Comments

Krzysztof Kozlowski April 22, 2024, 7:12 a.m. UTC | #1
On 12/04/2024 15:05, Sumit Gupta wrote:
> MC SID and Broadbast channel register access is restricted for Guest VM.

Same typo

> In Tegra MC driver, consider both the regions as optional and skip
> access to restricted registers from Guest if a region is not present
> in Guest DT.
> 

...

>  
>  static inline u32 mc_readl(const struct tegra_mc *mc, unsigned long offset)
> diff --git a/drivers/memory/tegra/tegra186.c b/drivers/memory/tegra/tegra186.c
> index 1b3183951bfe..716582255eeb 100644
> --- a/drivers/memory/tegra/tegra186.c
> +++ b/drivers/memory/tegra/tegra186.c
> @@ -26,20 +26,16 @@
>  static int tegra186_mc_probe(struct tegra_mc *mc)
>  {
>  	struct platform_device *pdev = to_platform_device(mc->dev);
> +	struct resource *res;
>  	unsigned int i;
> -	char name[8];
> +	char name[14];

How is it relevant? I don't see this being used in your diff.


Best regards,
Krzysztof
Sumit Gupta April 22, 2024, 2:36 p.m. UTC | #2
> On 12/04/2024 15:05, Sumit Gupta wrote:
>> MC SID and Broadbast channel register access is restricted for Guest VM.
> 
> Same typo
> 
Thank you for catching. Will correct in v4.

>> In Tegra MC driver, consider both the regions as optional and skip
>> access to restricted registers from Guest if a region is not present
>> in Guest DT.
>>
> 
> ...
> 
>>
>>   static inline u32 mc_readl(const struct tegra_mc *mc, unsigned long offset)
>> diff --git a/drivers/memory/tegra/tegra186.c b/drivers/memory/tegra/tegra186.c
>> index 1b3183951bfe..716582255eeb 100644
>> --- a/drivers/memory/tegra/tegra186.c
>> +++ b/drivers/memory/tegra/tegra186.c
>> @@ -26,20 +26,16 @@
>>   static int tegra186_mc_probe(struct tegra_mc *mc)
>>   {
>>        struct platform_device *pdev = to_platform_device(mc->dev);
>> +     struct resource *res;
>>        unsigned int i;
>> -     char name[8];
>> +     char name[14];
> 
> How is it relevant? I don't see this being used in your diff.
> 
> 
> Best regards,
> Krzysztof
> 

Did this change for below warning coming with 'W=1'.

../drivers/memory/tegra/tegra186.c: In function tegra186_mc_probe:
../drivers/memory/tegra/tegra186.c:51:49: warning: %u directive output 
may be truncated writing between 1 and 10 bytes into a region of size 6 
[8;;https://gc
c.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wformat-truncation=-Wformat-truncation=8;;]
    51 |                 snprintf(name, sizeof(name), "ch%u", i);
       |                                                 ^~
../drivers/memory/tegra/tegra186.c:51:46: note: directive argument in 
the range [0, 4294967294]
    51 |                 snprintf(name, sizeof(name), "ch%u", i);
       |                                              ^~~~~~
../drivers/memory/tegra/tegra186.c:51:17: note: snprintf output between 
4 and 13 bytes into a destination of size 8
    51 |                 snprintf(name, sizeof(name), "ch%u", i);
       |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Thank you,
Sumit Gupta
Krzysztof Kozlowski April 23, 2024, 2:41 p.m. UTC | #3
On 22/04/2024 16:36, Sumit Gupta wrote:
> 
>> On 12/04/2024 15:05, Sumit Gupta wrote:
>>> MC SID and Broadbast channel register access is restricted for Guest VM.
>>
>> Same typo
>>
> Thank you for catching. Will correct in v4.
> 
>>> In Tegra MC driver, consider both the regions as optional and skip
>>> access to restricted registers from Guest if a region is not present
>>> in Guest DT.
>>>
>>
>> ...
>>
>>>
>>>   static inline u32 mc_readl(const struct tegra_mc *mc, unsigned long offset)
>>> diff --git a/drivers/memory/tegra/tegra186.c b/drivers/memory/tegra/tegra186.c
>>> index 1b3183951bfe..716582255eeb 100644
>>> --- a/drivers/memory/tegra/tegra186.c
>>> +++ b/drivers/memory/tegra/tegra186.c
>>> @@ -26,20 +26,16 @@
>>>   static int tegra186_mc_probe(struct tegra_mc *mc)
>>>   {
>>>        struct platform_device *pdev = to_platform_device(mc->dev);
>>> +     struct resource *res;
>>>        unsigned int i;
>>> -     char name[8];
>>> +     char name[14];
>>
>> How is it relevant? I don't see this being used in your diff.
>>
>>
>> Best regards,
>> Krzysztof
>>
> 
> Did this change for below warning coming with 'W=1'.
> 
> ../drivers/memory/tegra/tegra186.c: In function tegra186_mc_probe:
> ../drivers/memory/tegra/tegra186.c:51:49: warning: %u directive output 
> may be truncated writing between 1 and 10 bytes into a region of size 6 
> [8;;https://gc
> c.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wformat-truncation=-Wformat-truncation=8;;]
>     51 |                 snprintf(name, sizeof(name), "ch%u", i);
>        |                                                 ^~
> ../drivers/memory/tegra/tegra186.c:51:46: note: directive argument in 
> the range [0, 4294967294]
>     51 |                 snprintf(name, sizeof(name), "ch%u", i);
>        |                                              ^~~~~~
> ../drivers/memory/tegra/tegra186.c:51:17: note: snprintf output between 
> 4 and 13 bytes into a destination of size 8
>     51 |                 snprintf(name, sizeof(name), "ch%u", i);
>        |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

I asked how this is relevant to this change and you answer there is a
warning. If the warning was there, your answer is really just deflecting
the topic, so obviously this is new warning. Which part of code uses
longer name?

BTW, really, such answers do not make review of your code smoother.

Best regards,
Krzysztof
Sumit Gupta April 23, 2024, 7:46 p.m. UTC | #4
>>>>
>>>>    static inline u32 mc_readl(const struct tegra_mc *mc, unsigned long offset)
>>>> diff --git a/drivers/memory/tegra/tegra186.c b/drivers/memory/tegra/tegra186.c
>>>> index 1b3183951bfe..716582255eeb 100644
>>>> --- a/drivers/memory/tegra/tegra186.c
>>>> +++ b/drivers/memory/tegra/tegra186.c
>>>> @@ -26,20 +26,16 @@
>>>>    static int tegra186_mc_probe(struct tegra_mc *mc)
>>>>    {
>>>>         struct platform_device *pdev = to_platform_device(mc->dev);
>>>> +     struct resource *res;
>>>>         unsigned int i;
>>>> -     char name[8];
>>>> +     char name[14];
>>>
>>> How is it relevant? I don't see this being used in your diff.
>>>
>>>
>>> Best regards,
>>> Krzysztof
>>>
>>
>> Did this change for below warning coming with 'W=1'.
>>
>> ../drivers/memory/tegra/tegra186.c: In function tegra186_mc_probe:
>> ../drivers/memory/tegra/tegra186.c:51:49: warning: %u directive output
>> may be truncated writing between 1 and 10 bytes into a region of size 6
>> [8;;https://gc
>> c.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wformat-truncation=-Wformat-truncation=8;;]
>>      51 |                 snprintf(name, sizeof(name), "ch%u", i);
>>         |                                                 ^~
>> ../drivers/memory/tegra/tegra186.c:51:46: note: directive argument in
>> the range [0, 4294967294]
>>      51 |                 snprintf(name, sizeof(name), "ch%u", i);
>>         |                                              ^~~~~~
>> ../drivers/memory/tegra/tegra186.c:51:17: note: snprintf output between
>> 4 and 13 bytes into a destination of size 8
>>      51 |                 snprintf(name, sizeof(name), "ch%u", i);
>>         |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> I asked how this is relevant to this change and you answer there is a
> warning. If the warning was there, your answer is really just deflecting
> the topic, so obviously this is new warning. Which part of code uses
> longer name?
> 
> BTW, really, such answers do not make review of your code smoother.
> 
> Best regards,
> Krzysztof
> 

Apologies for not explaining it earlier.

I increased the buffer size to suppress a static check warning in the
existing code due to big range of 'unsigned int i', if copied to small
name buffer.

Seems like the warning is harmless as the maximum value of num_channels
is 16. I will remove it and keep the buffer size as 8 in the next
version.

Thank you,
Sumit Gupta
Krzysztof Kozlowski April 24, 2024, 4:09 a.m. UTC | #5
On 23/04/2024 21:46, Sumit Gupta wrote:
> 
> 
> 
>>>>>
>>>>>    static inline u32 mc_readl(const struct tegra_mc *mc, unsigned long offset)
>>>>> diff --git a/drivers/memory/tegra/tegra186.c b/drivers/memory/tegra/tegra186.c
>>>>> index 1b3183951bfe..716582255eeb 100644
>>>>> --- a/drivers/memory/tegra/tegra186.c
>>>>> +++ b/drivers/memory/tegra/tegra186.c
>>>>> @@ -26,20 +26,16 @@
>>>>>    static int tegra186_mc_probe(struct tegra_mc *mc)
>>>>>    {
>>>>>         struct platform_device *pdev = to_platform_device(mc->dev);
>>>>> +     struct resource *res;
>>>>>         unsigned int i;
>>>>> -     char name[8];
>>>>> +     char name[14];
>>>>
>>>> How is it relevant? I don't see this being used in your diff.
>>>>
>>>>
>>>> Best regards,
>>>> Krzysztof
>>>>
>>>
>>> Did this change for below warning coming with 'W=1'.
>>>
>>> ../drivers/memory/tegra/tegra186.c: In function tegra186_mc_probe:
>>> ../drivers/memory/tegra/tegra186.c:51:49: warning: %u directive output
>>> may be truncated writing between 1 and 10 bytes into a region of size 6
>>> [8;;https://gc
>>> c.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wformat-truncation=-Wformat-truncation=8;;]
>>>      51 |                 snprintf(name, sizeof(name), "ch%u", i);
>>>         |                                                 ^~
>>> ../drivers/memory/tegra/tegra186.c:51:46: note: directive argument in
>>> the range [0, 4294967294]
>>>      51 |                 snprintf(name, sizeof(name), "ch%u", i);
>>>         |                                              ^~~~~~
>>> ../drivers/memory/tegra/tegra186.c:51:17: note: snprintf output between
>>> 4 and 13 bytes into a destination of size 8
>>>      51 |                 snprintf(name, sizeof(name), "ch%u", i);
>>>         |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>
>> I asked how this is relevant to this change and you answer there is a
>> warning. If the warning was there, your answer is really just deflecting
>> the topic, so obviously this is new warning. Which part of code uses
>> longer name?
>>
>> BTW, really, such answers do not make review of your code smoother.
>>
>> Best regards,
>> Krzysztof
>>
> 
> Apologies for not explaining it earlier.
> 
> I increased the buffer size to suppress a static check warning in the
> existing code due to big range of 'unsigned int i', if copied to small
> name buffer.
> 
> Seems like the warning is harmless as the maximum value of num_channels
> is 16. I will remove it and keep the buffer size as 8 in the next
> version.
> 

That's not the point. For the third time: how is it relevant to this
change here? Was or was not the warning before?

Best regards,
Krzysztof
Sumit Gupta April 24, 2024, 5:27 a.m. UTC | #6
>>
>>>>>>
>>>>>>     static inline u32 mc_readl(const struct tegra_mc *mc, unsigned long offset)
>>>>>> diff --git a/drivers/memory/tegra/tegra186.c b/drivers/memory/tegra/tegra186.c
>>>>>> index 1b3183951bfe..716582255eeb 100644
>>>>>> --- a/drivers/memory/tegra/tegra186.c
>>>>>> +++ b/drivers/memory/tegra/tegra186.c
>>>>>> @@ -26,20 +26,16 @@
>>>>>>     static int tegra186_mc_probe(struct tegra_mc *mc)
>>>>>>     {
>>>>>>          struct platform_device *pdev = to_platform_device(mc->dev);
>>>>>> +     struct resource *res;
>>>>>>          unsigned int i;
>>>>>> -     char name[8];
>>>>>> +     char name[14];
>>>>>
>>>>> How is it relevant? I don't see this being used in your diff.
>>>>>
>>>>>
>>>>> Best regards,
>>>>> Krzysztof
>>>>>
>>>>
>>>> Did this change for below warning coming with 'W=1'.
>>>>
>>>> ../drivers/memory/tegra/tegra186.c: In function tegra186_mc_probe:
>>>> ../drivers/memory/tegra/tegra186.c:51:49: warning: %u directive output
>>>> may be truncated writing between 1 and 10 bytes into a region of size 6
>>>> [8;;https://gc
>>>> c.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wformat-truncation=-Wformat-truncation=8;;]
>>>>       51 |                 snprintf(name, sizeof(name), "ch%u", i);
>>>>          |                                                 ^~
>>>> ../drivers/memory/tegra/tegra186.c:51:46: note: directive argument in
>>>> the range [0, 4294967294]
>>>>       51 |                 snprintf(name, sizeof(name), "ch%u", i);
>>>>          |                                              ^~~~~~
>>>> ../drivers/memory/tegra/tegra186.c:51:17: note: snprintf output between
>>>> 4 and 13 bytes into a destination of size 8
>>>>       51 |                 snprintf(name, sizeof(name), "ch%u", i);
>>>>          |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>>
>>> I asked how this is relevant to this change and you answer there is a
>>> warning. If the warning was there, your answer is really just deflecting
>>> the topic, so obviously this is new warning. Which part of code uses
>>> longer name?
>>>
>>> BTW, really, such answers do not make review of your code smoother.
>>>
>>> Best regards,
>>> Krzysztof
>>>
>>
>> Apologies for not explaining it earlier.
>>
>> I increased the buffer size to suppress a static check warning in the
>> existing code due to big range of 'unsigned int i', if copied to small
>> name buffer.
>>
>> Seems like the warning is harmless as the maximum value of num_channels
>> is 16. I will remove it and keep the buffer size as 8 in the next
>> version.
>>
> 
> That's not the point. For the third time: how is it relevant to this
> change here? Was or was not the warning before?
> 

This is not relevant to the change here. The warning was before as well.

Thank you,
Sumit Gupta

> Best regards,
> Krzysztof
>
Krzysztof Kozlowski April 24, 2024, 5:44 a.m. UTC | #7
On 24/04/2024 07:27, Sumit Gupta wrote:
> 
> 
>>>
>>>>>>>
>>>>>>>     static inline u32 mc_readl(const struct tegra_mc *mc, unsigned long offset)
>>>>>>> diff --git a/drivers/memory/tegra/tegra186.c b/drivers/memory/tegra/tegra186.c
>>>>>>> index 1b3183951bfe..716582255eeb 100644
>>>>>>> --- a/drivers/memory/tegra/tegra186.c
>>>>>>> +++ b/drivers/memory/tegra/tegra186.c
>>>>>>> @@ -26,20 +26,16 @@
>>>>>>>     static int tegra186_mc_probe(struct tegra_mc *mc)
>>>>>>>     {
>>>>>>>          struct platform_device *pdev = to_platform_device(mc->dev);
>>>>>>> +     struct resource *res;
>>>>>>>          unsigned int i;
>>>>>>> -     char name[8];
>>>>>>> +     char name[14];
>>>>>>
>>>>>> How is it relevant? I don't see this being used in your diff.
>>>>>>
>>>>>>
>>>>>> Best regards,
>>>>>> Krzysztof
>>>>>>
>>>>>
>>>>> Did this change for below warning coming with 'W=1'.
>>>>>
>>>>> ../drivers/memory/tegra/tegra186.c: In function tegra186_mc_probe:
>>>>> ../drivers/memory/tegra/tegra186.c:51:49: warning: %u directive output
>>>>> may be truncated writing between 1 and 10 bytes into a region of size 6
>>>>> [8;;https://gc
>>>>> c.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wformat-truncation=-Wformat-truncation=8;;]
>>>>>       51 |                 snprintf(name, sizeof(name), "ch%u", i);
>>>>>          |                                                 ^~
>>>>> ../drivers/memory/tegra/tegra186.c:51:46: note: directive argument in
>>>>> the range [0, 4294967294]
>>>>>       51 |                 snprintf(name, sizeof(name), "ch%u", i);
>>>>>          |                                              ^~~~~~
>>>>> ../drivers/memory/tegra/tegra186.c:51:17: note: snprintf output between
>>>>> 4 and 13 bytes into a destination of size 8
>>>>>       51 |                 snprintf(name, sizeof(name), "ch%u", i);
>>>>>          |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>>>
>>>> I asked how this is relevant to this change and you answer there is a
>>>> warning. If the warning was there, your answer is really just deflecting
>>>> the topic, so obviously this is new warning. Which part of code uses
>>>> longer name?
>>>>
>>>> BTW, really, such answers do not make review of your code smoother.
>>>>
>>>> Best regards,
>>>> Krzysztof
>>>>
>>>
>>> Apologies for not explaining it earlier.
>>>
>>> I increased the buffer size to suppress a static check warning in the
>>> existing code due to big range of 'unsigned int i', if copied to small
>>> name buffer.
>>>
>>> Seems like the warning is harmless as the maximum value of num_channels
>>> is 16. I will remove it and keep the buffer size as 8 in the next
>>> version.
>>>
>>
>> That's not the point. For the third time: how is it relevant to this
>> change here? Was or was not the warning before?
>>
> 
> This is not relevant to the change here. The warning was before as well.

OK, fixing the warning is always a good idea, but this *must* be always
separate patch, with its own explanation and rationale, and warning message.

Best regards,
Krzysztof
Sumit Gupta April 24, 2024, 6:27 a.m. UTC | #8
>>>>>>>>
>>>>>>>>      static inline u32 mc_readl(const struct tegra_mc *mc, unsigned long offset)
>>>>>>>> diff --git a/drivers/memory/tegra/tegra186.c b/drivers/memory/tegra/tegra186.c
>>>>>>>> index 1b3183951bfe..716582255eeb 100644
>>>>>>>> --- a/drivers/memory/tegra/tegra186.c
>>>>>>>> +++ b/drivers/memory/tegra/tegra186.c
>>>>>>>> @@ -26,20 +26,16 @@
>>>>>>>>      static int tegra186_mc_probe(struct tegra_mc *mc)
>>>>>>>>      {
>>>>>>>>           struct platform_device *pdev = to_platform_device(mc->dev);
>>>>>>>> +     struct resource *res;
>>>>>>>>           unsigned int i;
>>>>>>>> -     char name[8];
>>>>>>>> +     char name[14];
>>>>>>>
>>>>>>> How is it relevant? I don't see this being used in your diff.
>>>>>>>
>>>>>>>
>>>>>>> Best regards,
>>>>>>> Krzysztof
>>>>>>>
>>>>>>
>>>>>> Did this change for below warning coming with 'W=1'.
>>>>>>
>>>>>> ../drivers/memory/tegra/tegra186.c: In function tegra186_mc_probe:
>>>>>> ../drivers/memory/tegra/tegra186.c:51:49: warning: %u directive output
>>>>>> may be truncated writing between 1 and 10 bytes into a region of size 6
>>>>>> [8;;https://gc
>>>>>> c.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wformat-truncation=-Wformat-truncation=8;;]
>>>>>>        51 |                 snprintf(name, sizeof(name), "ch%u", i);
>>>>>>           |                                                 ^~
>>>>>> ../drivers/memory/tegra/tegra186.c:51:46: note: directive argument in
>>>>>> the range [0, 4294967294]
>>>>>>        51 |                 snprintf(name, sizeof(name), "ch%u", i);
>>>>>>           |                                              ^~~~~~
>>>>>> ../drivers/memory/tegra/tegra186.c:51:17: note: snprintf output between
>>>>>> 4 and 13 bytes into a destination of size 8
>>>>>>        51 |                 snprintf(name, sizeof(name), "ch%u", i);
>>>>>>           |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>>>>
>>>>> I asked how this is relevant to this change and you answer there is a
>>>>> warning. If the warning was there, your answer is really just deflecting
>>>>> the topic, so obviously this is new warning. Which part of code uses
>>>>> longer name?
>>>>>
>>>>> BTW, really, such answers do not make review of your code smoother.
>>>>>
>>>>> Best regards,
>>>>> Krzysztof
>>>>>
>>>>
>>>> Apologies for not explaining it earlier.
>>>>
>>>> I increased the buffer size to suppress a static check warning in the
>>>> existing code due to big range of 'unsigned int i', if copied to small
>>>> name buffer.
>>>>
>>>> Seems like the warning is harmless as the maximum value of num_channels
>>>> is 16. I will remove it and keep the buffer size as 8 in the next
>>>> version.
>>>>
>>>
>>> That's not the point. For the third time: how is it relevant to this
>>> change here? Was or was not the warning before?
>>>
>>
>> This is not relevant to the change here. The warning was before as well.
> 
> OK, fixing the warning is always a good idea, but this *must* be always
> separate patch, with its own explanation and rationale, and warning message.
> 

Sure, will submit a separate patch for the warning and spin a v4 for 
this patch series after incorporating all review comments.

Thank you,
Sumit Gupta

> Best regards,
> Krzysztof
>
diff mbox series

Patch

diff --git a/drivers/memory/tegra/mc.c b/drivers/memory/tegra/mc.c
index 224b488794e5..d819dab1b223 100644
--- a/drivers/memory/tegra/mc.c
+++ b/drivers/memory/tegra/mc.c
@@ -899,6 +899,7 @@  static void tegra_mc_num_channel_enabled(struct tegra_mc *mc)
 
 static int tegra_mc_probe(struct platform_device *pdev)
 {
+	struct resource *res;
 	struct tegra_mc *mc;
 	u64 mask;
 	int err;
@@ -923,7 +924,13 @@  static int tegra_mc_probe(struct platform_device *pdev)
 	/* length of MC tick in nanoseconds */
 	mc->tick = 30;
 
-	mc->regs = devm_platform_ioremap_resource(pdev, 0);
+	if (mc->soc->num_channels) {
+		res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "sid");
+		if (res)
+			mc->regs = devm_ioremap_resource(&pdev->dev, res);
+	} else {
+		mc->regs = devm_platform_ioremap_resource(pdev, 0);
+	}
 	if (IS_ERR(mc->regs))
 		return PTR_ERR(mc->regs);
 
diff --git a/drivers/memory/tegra/mc.h b/drivers/memory/tegra/mc.h
index c3f6655bec60..7e7bd3e09cdc 100644
--- a/drivers/memory/tegra/mc.h
+++ b/drivers/memory/tegra/mc.h
@@ -112,25 +112,27 @@  icc_provider_to_tegra_mc(struct icc_provider *provider)
 static inline u32 mc_ch_readl(const struct tegra_mc *mc, int ch,
 			      unsigned long offset)
 {
-	if (!mc->bcast_ch_regs)
-		return 0;
-
-	if (ch == MC_BROADCAST_CHANNEL)
+	if (ch == MC_BROADCAST_CHANNEL) {
+		if (!mc->bcast_ch_regs)
+			return 0;
 		return readl_relaxed(mc->bcast_ch_regs + offset);
+	} else if (mc->ch_regs) {
+		return readl_relaxed(mc->ch_regs[ch] + offset);
+	}
 
-	return readl_relaxed(mc->ch_regs[ch] + offset);
+	return 0;
 }
 
 static inline void mc_ch_writel(const struct tegra_mc *mc, int ch,
 				u32 value, unsigned long offset)
 {
-	if (!mc->bcast_ch_regs)
-		return;
-
-	if (ch == MC_BROADCAST_CHANNEL)
+	if (ch == MC_BROADCAST_CHANNEL) {
+		if (!mc->bcast_ch_regs)
+			return;
 		writel_relaxed(value, mc->bcast_ch_regs + offset);
-	else
+	} else if (mc->ch_regs) {
 		writel_relaxed(value, mc->ch_regs[ch] + offset);
+	}
 }
 
 static inline u32 mc_readl(const struct tegra_mc *mc, unsigned long offset)
diff --git a/drivers/memory/tegra/tegra186.c b/drivers/memory/tegra/tegra186.c
index 1b3183951bfe..716582255eeb 100644
--- a/drivers/memory/tegra/tegra186.c
+++ b/drivers/memory/tegra/tegra186.c
@@ -26,20 +26,16 @@ 
 static int tegra186_mc_probe(struct tegra_mc *mc)
 {
 	struct platform_device *pdev = to_platform_device(mc->dev);
+	struct resource *res;
 	unsigned int i;
-	char name[8];
+	char name[14];
 	int err;
 
-	mc->bcast_ch_regs = devm_platform_ioremap_resource_byname(pdev, "broadcast");
-	if (IS_ERR(mc->bcast_ch_regs)) {
-		if (PTR_ERR(mc->bcast_ch_regs) == -EINVAL) {
-			dev_warn(&pdev->dev,
-				 "Broadcast channel is missing, please update your device-tree\n");
-			mc->bcast_ch_regs = NULL;
-			goto populate;
-		}
-
-		return PTR_ERR(mc->bcast_ch_regs);
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "broadcast");
+	if (res) {
+		mc->bcast_ch_regs = devm_ioremap_resource(&pdev->dev, res);
+		if (IS_ERR(mc->bcast_ch_regs))
+			return PTR_ERR(mc->bcast_ch_regs);
 	}
 
 	mc->ch_regs = devm_kcalloc(mc->dev, mc->soc->num_channels, sizeof(*mc->ch_regs),
@@ -55,7 +51,6 @@  static int tegra186_mc_probe(struct tegra_mc *mc)
 			return PTR_ERR(mc->ch_regs[i]);
 	}
 
-populate:
 	err = of_platform_populate(mc->dev->of_node, NULL, NULL, mc->dev);
 	if (err < 0)
 		return err;
@@ -121,6 +116,9 @@  static int tegra186_mc_probe_device(struct tegra_mc *mc, struct device *dev)
 	if (!tegra_dev_iommu_get_stream_id(dev, &sid))
 		return 0;
 
+	if (!mc->regs)
+		return 0;
+
 	while (!of_parse_phandle_with_args(dev->of_node, "interconnects", "#interconnect-cells",
 					   index, &args)) {
 		if (args.np == mc->dev->of_node && args.args_count != 0) {
@@ -146,6 +144,9 @@  static int tegra186_mc_resume(struct tegra_mc *mc)
 #if IS_ENABLED(CONFIG_IOMMU_API)
 	unsigned int i;
 
+	if (!mc->regs)
+		return 0;
+
 	for (i = 0; i < mc->soc->num_clients; i++) {
 		const struct tegra_mc_client *client = &mc->soc->clients[i];