Hi Shyam,
On Wed, Sep 11, 2024 at 05:24:02PM GMT, 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.
...
> diff --git a/drivers/i2c/busses/i2c-piix4.c b/drivers/i2c/busses/i2c-piix4.c
> index 2c2a466e2f85..174cce254e96 100644
> --- a/drivers/i2c/busses/i2c-piix4.c
> +++ b/drivers/i2c/busses/i2c-piix4.c
> @@ -153,8 +153,7 @@ 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)
I’m not entirely happy with this change, or the others above. If
someone runs a git bisect, they would be confused by not seeing
this change described in the commit log.
While it's true that the accepted line length is now 100
characters, the 80-character limit is still preferred (and
personally, I prefer 80, though that’s just my opinion).
This change doesn’t seem necessary, so please amend it along with
the others below in the next version.
Thanks,
Andi
> {
> if (mmio_cfg->use_mmio) {
> void __iomem *addr;
> @@ -192,9 +191,9 @@ 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 +204,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 +514,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 +587,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 +741,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 +763,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.
...
