diff mbox series

[v1,3/6] bus: mhi: Add MMIO region length to controller structure

Message ID 1618598825-18629-4-git-send-email-bbhatt@codeaurora.org (mailing list archive)
State Changes Requested
Delegated to: Kalle Valo
Headers show
Series BHI/BHIe improvements for MHI power purposes | expand

Commit Message

Bhaumik Bhatt April 16, 2021, 6:47 p.m. UTC
Make controller driver specify the MMIO register region length
for range checking of BHI or BHIe space. This can help validate
that offsets are in acceptable memory region or not and avoid any
boot-up issues due to BHI or BHIe memory accesses.

Signed-off-by: Bhaumik Bhatt <bbhatt@codeaurora.org>
---
 include/linux/mhi.h | 2 ++
 1 file changed, 2 insertions(+)

Comments

Jeffrey Hugo May 4, 2021, 5:28 p.m. UTC | #1
On 4/16/2021 12:47 PM, Bhaumik Bhatt wrote:
> Make controller driver specify the MMIO register region length
> for range checking of BHI or BHIe space. This can help validate
> that offsets are in acceptable memory region or not and avoid any
> boot-up issues due to BHI or BHIe memory accesses.
> 
> Signed-off-by: Bhaumik Bhatt <bbhatt@codeaurora.org>
> ---
>   include/linux/mhi.h | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/include/linux/mhi.h b/include/linux/mhi.h
> index 944aa3a..ad53b24 100644
> --- a/include/linux/mhi.h
> +++ b/include/linux/mhi.h
> @@ -293,6 +293,7 @@ struct mhi_controller_config {
>    * @mhi_dev: MHI device instance for the controller
>    * @debugfs_dentry: MHI controller debugfs directory
>    * @regs: Base address of MHI MMIO register space (required)
> + * @reg_len: Length of the MHI MMIO region (required)
>    * @bhi: Points to base of MHI BHI register space
>    * @bhie: Points to base of MHI BHIe register space
>    * @wake_db: MHI WAKE doorbell register address
> @@ -375,6 +376,7 @@ struct mhi_controller {
>   	struct mhi_device *mhi_dev;
>   	struct dentry *debugfs_dentry;
>   	void __iomem *regs;
> +	size_t reg_len;

Didn't Mani make sure this struct was packed using pahole when things 
were first upstreamed?  Feels like sticking this in the middle of a 
bunch of void * entries will add some padding.  Am I wrong?

>   	void __iomem *bhi;
>   	void __iomem *bhie;
>   	void __iomem *wake_db;
>
Bhaumik Bhatt May 4, 2021, 11:04 p.m. UTC | #2
Hi Jeff,

On 2021-05-04 10:28 AM, Jeffrey Hugo wrote:
> On 4/16/2021 12:47 PM, Bhaumik Bhatt wrote:
>> Make controller driver specify the MMIO register region length
>> for range checking of BHI or BHIe space. This can help validate
>> that offsets are in acceptable memory region or not and avoid any
>> boot-up issues due to BHI or BHIe memory accesses.
>> 
>> Signed-off-by: Bhaumik Bhatt <bbhatt@codeaurora.org>
>> ---
>>   include/linux/mhi.h | 2 ++
>>   1 file changed, 2 insertions(+)
>> 
>> diff --git a/include/linux/mhi.h b/include/linux/mhi.h
>> index 944aa3a..ad53b24 100644
>> --- a/include/linux/mhi.h
>> +++ b/include/linux/mhi.h
>> @@ -293,6 +293,7 @@ struct mhi_controller_config {
>>    * @mhi_dev: MHI device instance for the controller
>>    * @debugfs_dentry: MHI controller debugfs directory
>>    * @regs: Base address of MHI MMIO register space (required)
>> + * @reg_len: Length of the MHI MMIO region (required)
>>    * @bhi: Points to base of MHI BHI register space
>>    * @bhie: Points to base of MHI BHIe register space
>>    * @wake_db: MHI WAKE doorbell register address
>> @@ -375,6 +376,7 @@ struct mhi_controller {
>>   	struct mhi_device *mhi_dev;
>>   	struct dentry *debugfs_dentry;
>>   	void __iomem *regs;
>> +	size_t reg_len;
> 
> Didn't Mani make sure this struct was packed using pahole when things
> were first upstreamed?  Feels like sticking this in the middle of a
> bunch of void * entries will add some padding.  Am I wrong?
> 
OK. I missed out on this detail. I can move this size_t entry in v2.
>>   	void __iomem *bhi;
>>   	void __iomem *bhie;
>>   	void __iomem *wake_db;
>> 

Thanks,
Bhaumik
---
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora 
Forum,
a Linux Foundation Collaborative Project
diff mbox series

Patch

diff --git a/include/linux/mhi.h b/include/linux/mhi.h
index 944aa3a..ad53b24 100644
--- a/include/linux/mhi.h
+++ b/include/linux/mhi.h
@@ -293,6 +293,7 @@  struct mhi_controller_config {
  * @mhi_dev: MHI device instance for the controller
  * @debugfs_dentry: MHI controller debugfs directory
  * @regs: Base address of MHI MMIO register space (required)
+ * @reg_len: Length of the MHI MMIO region (required)
  * @bhi: Points to base of MHI BHI register space
  * @bhie: Points to base of MHI BHIe register space
  * @wake_db: MHI WAKE doorbell register address
@@ -375,6 +376,7 @@  struct mhi_controller {
 	struct mhi_device *mhi_dev;
 	struct dentry *debugfs_dentry;
 	void __iomem *regs;
+	size_t reg_len;
 	void __iomem *bhi;
 	void __iomem *bhie;
 	void __iomem *wake_db;