diff mbox series

[v5,3/8] i2c: piix4: Export i2c_piix4 driver functions as library

Message ID 20240913121110.1611340-4-Shyam-sundar.S-k@amd.com
State New
Headers show
Series Introduce initial AMD ASF Controller driver support | expand

Commit Message

Shyam Sundar S K Sept. 13, 2024, 12:11 p.m. UTC 
Export the following i2c_piix4 driver functions as a library so that the
AMD ASF driver can utilize these core functionalities from the i2c_piix4
driver:

- piix4_sb800_region_request(): Request access to a specific SMBus region
on the SB800 chipset.

- piix4_sb800_region_release(): Release the previously requested SMBus
region on the SB800 chipset.

- piix4_transaction(): Handle SMBus transactions between the SMBus
controller and connected devices.

- piix4_sb800_port_sel(): Select the appropriate SMBus port on the SB800
chipset.

By making these functions available as a library, enable the AMD ASF
driver to leverage the established mechanisms in the i2c_piix4 driver,
promoting code reuse and consistency across different drivers.

Note that the git diff view is presented in two separate lines in order to
suppress the checkpatch.pl "CHECKS".

Co-developed-by: Sanket Goswami <Sanket.Goswami@amd.com>
Signed-off-by: Sanket Goswami <Sanket.Goswami@amd.com>
Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
---
 drivers/i2c/busses/i2c-piix4.c | 16 ++++++++++------
 drivers/i2c/busses/i2c-piix4.h |  5 +++++
 2 files changed, 15 insertions(+), 6 deletions(-)

Comments

Andy Shevchenko Sept. 13, 2024, 6:54 p.m. UTC | #1 
On Fri, Sep 13, 2024 at 05:41:05PM +0530, Shyam Sundar S K wrote:
> Export the following i2c_piix4 driver functions as a library so that the
> AMD ASF driver can utilize these core functionalities from the i2c_piix4
> driver:
> 
> - piix4_sb800_region_request(): Request access to a specific SMBus region
> on the SB800 chipset.
> 
> - piix4_sb800_region_release(): Release the previously requested SMBus
> region on the SB800 chipset.
> 
> - piix4_transaction(): Handle SMBus transactions between the SMBus
> controller and connected devices.
> 
> - piix4_sb800_port_sel(): Select the appropriate SMBus port on the SB800
> chipset.
> 
> By making these functions available as a library, enable the AMD ASF
> driver to leverage the established mechanisms in the i2c_piix4 driver,
> promoting code reuse and consistency across different drivers.

> Note that the git diff view is presented in two separate lines in order to
> suppress the checkpatch.pl "CHECKS".

This paragraph should be in comment block rather than commit message body...

> Co-developed-by: Sanket Goswami <Sanket.Goswami@amd.com>
> Signed-off-by: Sanket Goswami <Sanket.Goswami@amd.com>
> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
> ---

...somewhere here.

...

> +int piix4_sb800_region_request(struct device *dev,
> +			       struct sb800_mmio_cfg *mmio_cfg)

One line?

...

> +EXPORT_SYMBOL_GPL(piix4_sb800_region_request);

Use namespaced exports (with _NS) from day 1.

...

> +void piix4_sb800_region_release(struct device *dev,
> +				struct sb800_mmio_cfg *mmio_cfg)

> +EXPORT_SYMBOL_GPL(piix4_sb800_region_release);

Same comments as per above.

...

> +EXPORT_SYMBOL_GPL(piix4_transaction);
> +EXPORT_SYMBOL_GPL(piix4_sb800_port_sel);

_NS
Shyam Sundar S K Sept. 17, 2024, 6:14 p.m. UTC | #2 
On 9/14/2024 00:24, Andy Shevchenko wrote:
> On Fri, Sep 13, 2024 at 05:41:05PM +0530, Shyam Sundar S K wrote:
>> Export the following i2c_piix4 driver functions as a library so that the
>> AMD ASF driver can utilize these core functionalities from the i2c_piix4
>> driver:
>>
>> - piix4_sb800_region_request(): Request access to a specific SMBus region
>> on the SB800 chipset.
>>
>> - piix4_sb800_region_release(): Release the previously requested SMBus
>> region on the SB800 chipset.
>>
>> - piix4_transaction(): Handle SMBus transactions between the SMBus
>> controller and connected devices.
>>
>> - piix4_sb800_port_sel(): Select the appropriate SMBus port on the SB800
>> chipset.
>>
>> By making these functions available as a library, enable the AMD ASF
>> driver to leverage the established mechanisms in the i2c_piix4 driver,
>> promoting code reuse and consistency across different drivers.
> 
>> Note that the git diff view is presented in two separate lines in order to
>> suppress the checkpatch.pl "CHECKS".
> 
> This paragraph should be in comment block rather than commit message body...
> 

I can move it to comment block but in the last version Andi mentioned
that I have to leave a note about the function within one line.

>> Co-developed-by: Sanket Goswami <Sanket.Goswami@amd.com>
>> Signed-off-by: Sanket Goswami <Sanket.Goswami@amd.com>
>> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
>> ---
> 
> ...somewhere here.
> 
> ...
> 
>> +int piix4_sb800_region_request(struct device *dev,
>> +			       struct sb800_mmio_cfg *mmio_cfg)
> 
> One line?
> 

I am OK to do it, but Andi has a preference to stay within 80
character wide length.

Andi, what are you thoughts?

Thanks,
Shyam

> ...
> 
>> +EXPORT_SYMBOL_GPL(piix4_sb800_region_request);
> 
> Use namespaced exports (with _NS) from day 1.
> 
> ...
> 
>> +void piix4_sb800_region_release(struct device *dev,
>> +				struct sb800_mmio_cfg *mmio_cfg)
> 
>> +EXPORT_SYMBOL_GPL(piix4_sb800_region_release);
> 
> Same comments as per above.
> 
> ...
> 
>> +EXPORT_SYMBOL_GPL(piix4_transaction);
>> +EXPORT_SYMBOL_GPL(piix4_sb800_port_sel);
> 
> _NS
>
Andy Shevchenko Sept. 18, 2024, 9:56 a.m. UTC | #3 
On Tue, Sep 17, 2024 at 11:44:04PM +0530, Shyam Sundar S K wrote:
> On 9/14/2024 00:24, Andy Shevchenko wrote:
> > On Fri, Sep 13, 2024 at 05:41:05PM +0530, Shyam Sundar S K wrote:

...

> >> Note that the git diff view is presented in two separate lines in order to
> >> suppress the checkpatch.pl "CHECKS".

(1) ^^^

> > This paragraph should be in comment block rather than commit message body...
> 
> I can move it to comment block but in the last version Andi mentioned
> that I have to leave a note about the function within one line.

What function? I'm talking about (1) which refers to Git and checkpatch.
Shyam Sundar S K Sept. 18, 2024, 10:14 a.m. UTC | #4 
On 9/18/2024 15:26, Andy Shevchenko wrote:
> On Tue, Sep 17, 2024 at 11:44:04PM +0530, Shyam Sundar S K wrote:
>> On 9/14/2024 00:24, Andy Shevchenko wrote:
>>> On Fri, Sep 13, 2024 at 05:41:05PM +0530, Shyam Sundar S K wrote:
> 
> ...
> 
>>>> Note that the git diff view is presented in two separate lines in order to
>>>> suppress the checkpatch.pl "CHECKS".
> 
> (1) ^^^
> 
>>> This paragraph should be in comment block rather than commit message body...
>>
>> I can move it to comment block but in the last version Andi mentioned
>> that I have to leave a note about the function within one line.
> 
> What function? I'm talking about (1) which refers to Git and checkpatch.
> 

There was a remark on this function to make it into a single line.

+int piix4_sb800_region_request(struct device *dev,
+			       struct sb800_mmio_cfg *mmio_cfg)


Anyways, let me not confuse more. I will put the paragraph what you
pointed in the comment block.

Thanks,
Shyam
Andi Shyti Sept. 18, 2024, 8:50 p.m. UTC | #5 
Hi Shyam,

...

> >> Note that the git diff view is presented in two separate lines in order to
> >> suppress the checkpatch.pl "CHECKS".
> > 
> > This paragraph should be in comment block rather than commit message body...
> > 
> 
> I can move it to comment block but in the last version Andi mentioned
> that I have to leave a note about the function within one line.
> 
> >> Co-developed-by: Sanket Goswami <Sanket.Goswami@amd.com>
> >> Signed-off-by: Sanket Goswami <Sanket.Goswami@amd.com>
> >> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
> >> ---
> > 
> > ...somewhere here.
> > 
> > ...
> > 
> >> +int piix4_sb800_region_request(struct device *dev,
> >> +			       struct sb800_mmio_cfg *mmio_cfg)
> > 
> > One line?
> > 
> 
> I am OK to do it, but Andi has a preference to stay within 80
> character wide length.
> 
> Andi, what are you thoughts?

I apologize for my earlier review of v4. I failed to notice that
you had removed the "static" (which you had properly described).
I mistakenly thought you had made an unrelated line adjustment.

I'm sorry for requesting a new version. Please feel free to use
the format you prefer.

Andi

PS: I still prefer 80 characters per line, but this is just a
personal, non-binding preference.
diff mbox series

Patch

diff --git a/drivers/i2c/busses/i2c-piix4.c b/drivers/i2c/busses/i2c-piix4.c
index 2c2a466e2f85..7e0fb51ce532 100644
--- a/drivers/i2c/busses/i2c-piix4.c
+++ b/drivers/i2c/busses/i2c-piix4.c
@@ -153,8 +153,8 @@  struct i2c_piix4_adapdata {
 	struct sb800_mmio_cfg mmio_cfg;
 };
 
-static int piix4_sb800_region_request(struct device *dev,
-				      struct sb800_mmio_cfg *mmio_cfg)
+int piix4_sb800_region_request(struct device *dev,
+			       struct sb800_mmio_cfg *mmio_cfg)
 {
 	if (mmio_cfg->use_mmio) {
 		void __iomem *addr;
@@ -192,9 +192,10 @@  static int piix4_sb800_region_request(struct device *dev,
 
 	return 0;
 }
+EXPORT_SYMBOL_GPL(piix4_sb800_region_request);
 
-static void piix4_sb800_region_release(struct device *dev,
-				       struct sb800_mmio_cfg *mmio_cfg)
+void piix4_sb800_region_release(struct device *dev,
+				struct sb800_mmio_cfg *mmio_cfg)
 {
 	if (mmio_cfg->use_mmio) {
 		iounmap(mmio_cfg->addr);
@@ -205,6 +206,7 @@  static void piix4_sb800_region_release(struct device *dev,
 
 	release_region(SB800_PIIX4_SMB_IDX, SB800_PIIX4_SMB_MAP_SIZE);
 }
+EXPORT_SYMBOL_GPL(piix4_sb800_region_release);
 
 static bool piix4_sb800_use_mmio(struct pci_dev *PIIX4_dev)
 {
@@ -514,7 +516,7 @@  static int piix4_setup_aux(struct pci_dev *PIIX4_dev,
 	return piix4_smba;
 }
 
-static int piix4_transaction(struct i2c_adapter *piix4_adapter, unsigned short piix4_smba)
+int piix4_transaction(struct i2c_adapter *piix4_adapter, unsigned short piix4_smba)
 {
 	int temp;
 	int result = 0;
@@ -587,6 +589,7 @@  static int piix4_transaction(struct i2c_adapter *piix4_adapter, unsigned short p
 		inb_p(SMBHSTDAT1));
 	return result;
 }
+EXPORT_SYMBOL_GPL(piix4_transaction);
 
 /* Return negative errno on error. */
 static s32 piix4_access(struct i2c_adapter * adap, u16 addr,
@@ -740,7 +743,7 @@  static void piix4_imc_wakeup(void)
 	release_region(KERNCZ_IMC_IDX, 2);
 }
 
-static int piix4_sb800_port_sel(u8 port, struct sb800_mmio_cfg *mmio_cfg)
+int piix4_sb800_port_sel(u8 port, struct sb800_mmio_cfg *mmio_cfg)
 {
 	u8 smba_en_lo, val;
 
@@ -762,6 +765,7 @@  static int piix4_sb800_port_sel(u8 port, struct sb800_mmio_cfg *mmio_cfg)
 
 	return (smba_en_lo & piix4_port_mask_sb800);
 }
+EXPORT_SYMBOL_GPL(piix4_sb800_port_sel);
 
 /*
  * Handles access to multiple SMBus ports on the SB800.
diff --git a/drivers/i2c/busses/i2c-piix4.h b/drivers/i2c/busses/i2c-piix4.h
index c4c20edacb00..9a5faac3eedd 100644
--- a/drivers/i2c/busses/i2c-piix4.h
+++ b/drivers/i2c/busses/i2c-piix4.h
@@ -37,4 +37,9 @@  struct sb800_mmio_cfg {
 	bool use_mmio;
 };
 
+int piix4_sb800_port_sel(u8 port, struct sb800_mmio_cfg *mmio_cfg);
+int piix4_transaction(struct i2c_adapter *piix4_adapter, unsigned short piix4_smba);
+int piix4_sb800_region_request(struct device *dev, struct sb800_mmio_cfg *mmio_cfg);
+void piix4_sb800_region_release(struct device *dev, struct sb800_mmio_cfg *mmio_cfg);
+
 #endif /* I2C_PIIX4_H */