IOSF: Add interface for the cases requiring fid
PATCH 1/1 IOSF: Add interface for the cases requiring fid
From: Fei Yang <[email protected]>
Some implementations may require an additional step for setting the FID bits to ensure correct transactions over the IOSF side band interface. Add the FID support accordingly for such implementations
--- arch/x86/include/asm/iosf_mbi.h | 27 ++++++++++++++ arch/x86/platform/intel/iosf_mbi.c | 73 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 100 insertions(+)
arch/x86/include/asm/iosf_mbi.h +27 -0
@@ -8,6 +8,7 @@#define MBI_MCR_OFFSET 0xD0#define MBI_MDR_OFFSET 0xD4#define MBI_MCRX_OFFSET 0xD8 +#define MBI_MCRP_OFFSET 0xDC#define MBI_RD_MASK 0xFEFFFFFF#define MBI_WR_MASK 0X01000000
@@ -88,6 +89,32 @@ int iosf_mbi_write(u8 port, u8 opcode, u32 offset, u32 mdr);*/ int iosf_mbi_modify (u8 port,u8 opcode,u32 offset,u32 mdr,u32 mask); +/** +* iosf_mbi_read_with_fid() - MailBox Interface read command requiring fid +* @fid: fid bits +* @port: port indicating subunit being accessed +* @opcode: port specific read or write opcode +* @offset: register address offset +* @mdr: register data to be read +* +* Locking is handled by spinlock - cannot sleep. +* Return: Nonzero on error +*/ +int iosf_mbi_read_with_fid (u32 fid,u8 port,u8 opcode,u32 offset,u32 *mdr); + +/** +* iosf_mbi_write_with_fid() - MailBox unmasked write command requiring fid +* @fid: fid bits +* @port: port indicating subunit being accessed +* @opcode: port specific read or write opcode +* @offset: register address offset +* @mdr: register data to be written +* +* Locking is handled by spinlock - cannot sleep. +* Return: Nonzero on error +*/ +int iosf_mbi_write_with_fid (u32 fid,u8 port,u8 opcode,u32 offset,u32 mdr); +#else /* CONFIG_IOSF_MBI is not enabled */ static inline bool iosf_mbi_available (void )
arch/x86/platform/intel/iosf_mbi.c +73 -0
85 lines not on this page · raw message
Signature · 1 line
1.9.1
Quoted 2 lines · Fei Yang
From: Fei Yang <[email protected]>
In subject better to use x86/platform/iosf_mbi prefix.
Quoted 2 lines
Some implementations may require an additional step for setting the FID
What FID stands for?
Quoted 6 lines
bits to ensure correct transactions over the IOSF side band interface. Add the FID support accordingly for such implementations
Change-Id: Ic0227f9e74133a3203aa08e8471939f905d19622
This should not be here.
Quoted 50 lines
arch/x86/include/asm/iosf_mbi.h +0
-0@@ -88,6 +89,32 @@ int iosf_mbi_write(u8 port, u8 opcode, u32 offset,u32 mdr); */ int
iosf_mbi_modify(u8 port, u8 opcode, u32 offset, u32 mdr, u32 mask);
+/**
+ *iosf_mbi_read_with_fid()- MailBox Interface read command requiring fid
+ * @fid: fid bits
+ * @port: port indicating subunit being accessed
+ * @opcode: port specific read or write opcode
+ * @offset: register address offset
+ * @mdr: register data to be read
+ *
+ * Locking is handled by spinlock - cannot sleep.
+ * Return: Nonzero on error
+ */
+intiosf_mbi_read_with_fid(u32 fid, u8 port, u8 opcode, u32 offset, u32 *mdr);arch/x86/include/asm/iosf_mbi.h +3
-0+ +/** +* iosf_mbi_write_with_fid() - MailBox unmasked write command requiring fid
+ * @fid: fid bits
+ * @port: port indicating subunit being accessed
+ * @opcode: port specific read or write opcode
+ * @offset: register address offset
+ * @mdr: register data to be written
+ *
+ * Locking is handled by spinlock - cannot sleep.
+ * Return: Nonzero on error
+ */
+intiosf_mbi_write_with_fid(u32 fid, u8 port, u8 opcode, u32 offset, u32 mdr);
+
#else /*CONFIG_IOSF_MBIis not enabled */
static inline
booliosf_mbi_available(void)arch/x86/platform/intel/iosf_mbi.c +0
-0@@ -98,6 +98,24 @@ fail_write: return result; }
Quoted 2 lines
+static intiosf_mbi_pci_write_fid(u32 fid)
Function name should continue already used pattern, i.e. …_write_mcrp()
Quoted 10 lines
arch/x86/platform/intel/iosf_mbi.c +7
-0+{ +int result; + +if (!mbi_pdev) +return -ENODEV ; + + result = pci_write_config_dword(mbi_pdev, MBI_MCRP_OFFSET,fid);
The function of one line.
So, please, inline it directly where it's used.
Quoted 18 lines
arch/x86/platform/intel/iosf_mbi.c +7
-0+if (result <0 ) + goto fail_fid_write; + +return 0 ; + +fail_fid_write: + dev_err(&mbi_pdev->dev, "PCI config access failed with %d\n",result);
arch/x86/platform/intel/iosf_mbi.c +3
-0+return result; +} +int
iosf_mbi_read(u8 port, u8 opcode, u32 offset, u32 *mdr){
u32 mcr, mcrx;arch/x86/platform/intel/iosf_mbi.c +0
-0@@ -183,6 +201,61 @@ int iosf_mbi_modify(u8 port, u8 opcode, u32 offset, u32 mdr, u32 mask) } EXPORT_SYMBOL(iosf_mbi_modify);
Quoted 3 lines
+/*
+ * Some IP blocks require fid to access their registers.
Any user?
This API doesn't make much sense without user.
Quoted 5 lines
+ * fid value is programmed through MCRP register, see above function
+ *iosf_mbi_pci_write_fid()for details.
+ */
+intiosf_mbi_read_with_fid(u32 fid, u8 port, u8 opcode, u32 offset, u32 *mdr)
Name kinda fuzzy. How user should know which one to choose? Does fid == 0 work for some cases? We have to think about API and naming.
arch/x86/platform/intel/iosf_mbi.c int iosf_mbi_modify(u8 port, u8 opcode, u32 offset, u32 mdr, u32 mask) · line 215
Quoted 6 lines
arch/x86/platform/intel/iosf_mbi.c +6
-0+{ +u32 mcr, mcrx; +unsigned long flags; +int ret; + +/*Access to the GFX unit is handled by GPU code */
Spaces.
Quoted 20 lines
arch/x86/platform/intel/iosf_mbi.c +19
-0+if (port ==BT_MBI_UNIT_GFX ) { +WARN_ON (1 ); +return -EPERM ; + } + + mcr =iosf_mbi_form_mcr (opcode, port, offset &MBI_MASK_LO ); + mcrx = offset &MBI_MASK_HI ; + +spin_lock_irqsave (&iosf_mbi_lock, flags); + ret =iosf_mbi_pci_write_fid (fid); +if (!ret) + ret =iosf_mbi_pci_read_mdr (mcrx, mcr, mdr); +spin_unlock_irqrestore (&iosf_mbi_lock, flags); + +return ret; +} +EXPORT_SYMBOL (iosf_mbi_read_with_fid); + +int iosf_mbi_write_with_fid(u32 fid, u8 port, u8 opcode, u32 offset,u32 mdr)
arch/x86/platform/intel/iosf_mbi.c int iosf_mbi_modify(u8 port, u8 opcode, u32 offset, u32 mdr, u32 mask) · line 215
Quoted 6 lines
arch/x86/platform/intel/iosf_mbi.c +6
-0+{ +u32 mcr, mcrx; +unsigned long flags; +int ret; + +/*Access to the GFX unit is handled by GPU code */
Ditto.
arch/x86/platform/intel/iosf_mbi.c int iosf_mbi_modify(u8 port, u8 opcode, u32 offset, u32 mdr, u32 mask) · line 253
Quoted 13 lines
arch/x86/platform/intel/iosf_mbi.c +13
-0+if (port ==BT_MBI_UNIT_GFX ) { +WARN_ON (1 ); +return -EPERM ; + } + + mcr =iosf_mbi_form_mcr (opcode, port, offset &MBI_MASK_LO ); + mcrx = offset &MBI_MASK_HI ; + +spin_lock_irqsave (&iosf_mbi_lock, flags); + ret =iosf_mbi_pci_write_fid (fid); +if (!ret) + ret =iosf_mbi_pci_write_mdr (mcrx, mcr, mdr); +spin_unlock_irqrestore (&iosf_mbi_lock, flags);
Both of them quite similar to already exist _write()/_read(). Is it possible to combine them out?
Quoted 8 lines
arch/x86/platform/intel/iosf_mbi.c +5
-0+ +return ret; +} +EXPORT_SYMBOL (iosf_mbi_write_with_fid); +bool
iosf_mbi_available(void)
{
/* Mbi isn't hot-pluggable. No remove routine is provided */
Signature · 2 lines
Andy Shevchenko <[email protected]> Intel Finland Oy
Quoted 1 line
In subject better to use x86/platform/iosf_mbi prefix.
Will update commit message.
Quoted 3 lines
Quoted 2 lines
Some implementations may require an additional step for setting the FID
What FID stands for?
Function ID defined in IOSF specification. Will add more detail in the updated commit message.
Quoted 2 lines
Quoted 1 line
Change-Id: Ic0227f9e74133a3203aa08e8471939f905d19622
This should not be here.
Will remove
Quoted 9 lines
Quoted 7 lines
arch/x86/platform/intel/iosf_mbi.c +0
-0@@ -98,6 +98,24 @@ fail_write: return result; }
+static intiosf_mbi_pci_write_fid(u32 fid)Function name should continue already used pattern, i.e. …_write_mcrp()
Will change.
Quoted 8 lines
Quoted 6 lines
arch/x86/platform/intel/iosf_mbi.c +6
-0+{ +int result; + +if (!mbi_pdev) +return -ENODEV ; +
Quoted 5 lines
Quoted 2 lines
arch/x86/platform/intel/iosf_mbi.c +1
-0+ result = pci_write_config_dword(mbi_pdev, MBI_MCRP_OFFSET,fid);
The function of one line.
So, please, inline it directly where it's used.
Not sure I understand this one, do you meant something like this? static inline int iosf_mbi_pci_write_fid(u32 fid)
Quoted 4 lines
Quoted 2 lines
+/*
+ * Some IP blocks require fid to access their registers.Any user?
This API doesn't make much sense without user.
The driver that uses this API is a DRM based display controller driver, which is not currently in the upstream. But this API is a prerequisite of pushing the display driver upstream.
Quoted 7 lines
Quoted 5 lines
+ * fid value is programmed through MCRP register, see above function
+ *iosf_mbi_pci_write_fid()for details.
+ */
+intiosf_mbi_read_with_fid(u32 fid, u8 port, u8 opcode, u32 offset, u32 *mdr)Name kinda fuzzy. How user should know which one to choose? Does fid == 0 work for some cases? We have to think about API and naming.
This is SoC dependent. The drivers accessing registers through IOSF sideband has to know if fid is required.
Quoted 2 lines
arch/x86/platform/intel/iosf_mbi.c
Quoted 1 line
arch/x86/platform/intel/iosf_mbi.c +1
-0+/*Access to the GFX unit is handled by GPU code */ Spaces.
Will add.
Quoted 2 lines
arch/x86/platform/intel/iosf_mbi.c
Quoted 1 line
arch/x86/platform/intel/iosf_mbi.c +1
-0+/*Access to the GFX unit is handled by GPU code */ Ditto.
Will add.
Quoted 9 lines
arch/x86/platform/intel/iosf_mbi.c int iosf_mbi_modify(u8 port, u8 opcode, u32 offset, u32 mdr, u32 mask) · line 253
Quoted 8 lines
arch/x86/platform/intel/iosf_mbi.c +8
-0+ mcr =iosf_mbi_form_mcr (opcode, port, offset &MBI_MASK_LO ); + mcrx = offset &MBI_MASK_HI ; + +spin_lock_irqsave (&iosf_mbi_lock, flags); + ret =iosf_mbi_pci_write_fid (fid); +if (!ret) + ret =iosf_mbi_pci_write_mdr (mcrx, mcr, mdr); +spin_unlock_irqrestore (&iosf_mbi_lock, flags);Both of them quite similar to already exist _write()/_read(). Is it possible to combine them out?
Trying to avoid modifying existing code that uses the _read/_write API.
Quoted 7 lines · Fei Yang
From: Fei Yang <[email protected]>
Some implementations may require an additional step for setting the FID
bits to ensure correct transactions over the IOSF side band interface. Add the FID support accordingly for such implementations
Cc'ed to Ville to take a look.
As mentioned in a follow up the user of this is "a DRM based display controller driver, which is not currently in the upstream. But this API is a prerequisite of pushing the display driver upstream."
Quoted 157 lines
Change-Id Ic0227f9e74133a3203aa08e8471939f905d19622Signed-off-by Fei Yang <[email protected]>---
arch/x86/include/asm/iosf_mbi.h | 27 ++++++++++++++arch/x86/platform/intel/iosf_mbi.c| 73
++++++++++++++++++++++++++++++++++++++2 files changed, 100 insertions(+)arch/x86/include/asm/iosf_mbi.h +1
-0@@ -8,6 +8,7 @@ #define MBI_MCR_OFFSET 0xD0 #define MBI_MDR_OFFSET 0xD4 #define MBI_MCRX_OFFSET 0xD8 +#define MBI_MCRP_OFFSET 0xDC
#defineMBI_RD_MASK0xFEFFFFFF
#defineMBI_WR_MASK0X01000000arch/x86/include/asm/iosf_mbi.h +0
-0@@ -88,6 +89,32 @@ int iosf_mbi_write(u8 port, u8 opcode, u32 offset,u32 mdr); */ int
iosf_mbi_modify(u8 port, u8 opcode, u32 offset, u32 mdr, u32 mask);
+/**
+ *iosf_mbi_read_with_fid()- MailBox Interface read command requiring fid
+ * @fid: fid bits
+ * @port: port indicating subunit being accessed
+ * @opcode: port specific read or write opcode
+ * @offset: register address offset
+ * @mdr: register data to be read
+ *
+ * Locking is handled by spinlock - cannot sleep.
+ * Return: Nonzero on error
+ */
+intiosf_mbi_read_with_fid(u32 fid, u8 port, u8 opcode, u32 offset, u32 *mdr);arch/x86/include/asm/iosf_mbi.h +3
-0+ +/** +* iosf_mbi_write_with_fid() - MailBox unmasked write command requiring fid
+ * @fid: fid bits
+ * @port: port indicating subunit being accessed
+ * @opcode: port specific read or write opcode
+ * @offset: register address offset
+ * @mdr: register data to be written
+ *
+ * Locking is handled by spinlock - cannot sleep.
+ * Return: Nonzero on error
+ */
+intiosf_mbi_write_with_fid(u32 fid, u8 port, u8 opcode, u32 offset, u32 mdr);
+
#else /*CONFIG_IOSF_MBIis not enabled */
static inline
booliosf_mbi_available(void)arch/x86/platform/intel/iosf_mbi.c +0
-0@@ -98,6 +98,24 @@ fail_write: return result; }arch/x86/platform/intel/iosf_mbi.c +8
-0+static int iosf_mbi_pci_write_fid (u32 fid) +{ +int result; + +if (!mbi_pdev) +return -ENODEV ; + + result = pci_write_config_dword(mbi_pdev, MBI_MCRP_OFFSET,fid);
arch/x86/platform/intel/iosf_mbi.c +7
-0+if (result <0 ) + goto fail_fid_write; + +return 0 ; + +fail_fid_write: + dev_err(&mbi_pdev->dev, "PCI config access failed with %d\n",result);
arch/x86/platform/intel/iosf_mbi.c +3
-0+return result; +} +int
iosf_mbi_read(u8 port, u8 opcode, u32 offset, u32 *mdr){
u32 mcr, mcrx;arch/x86/platform/intel/iosf_mbi.c +0
-0@@ -183,6 +201,61 @@ int iosf_mbi_modify(u8 port, u8 opcode, u32 offset, u32 mdr, u32 mask) } EXPORT_SYMBOL(iosf_mbi_modify);arch/x86/platform/intel/iosf_mbi.c +6
-0+/* +* Some IP blocks require fid to access their registers. +* fid value is programmed through MCRP register, see above function +* iosf_mbi_pci_write_fid() for details. +*/ +int iosf_mbi_read_with_fid(u32 fid, u8 port, u8 opcode, u32 offset,u32 *mdr)
arch/x86/platform/intel/iosf_mbi.c +25
-0+{ +u32 mcr, mcrx; +unsigned long flags; +int ret; + +/*Access to the GFX unit is handled by GPU code */ +if (port ==BT_MBI_UNIT_GFX ) { +WARN_ON (1 ); +return -EPERM ; + } + + mcr =iosf_mbi_form_mcr (opcode, port, offset &MBI_MASK_LO ); + mcrx = offset &MBI_MASK_HI ; + +spin_lock_irqsave (&iosf_mbi_lock, flags); + ret =iosf_mbi_pci_write_fid (fid); +if (!ret) + ret =iosf_mbi_pci_read_mdr (mcrx, mcr, mdr); +spin_unlock_irqrestore (&iosf_mbi_lock, flags); + +return ret; +} +EXPORT_SYMBOL (iosf_mbi_read_with_fid); + +int iosf_mbi_write_with_fid(u32 fid, u8 port, u8 opcode, u32 offset,u32 mdr)
arch/x86/platform/intel/iosf_mbi.c +24
-0+{ +u32 mcr, mcrx; +unsigned long flags; +int ret; + +/*Access to the GFX unit is handled by GPU code */ +if (port ==BT_MBI_UNIT_GFX ) { +WARN_ON (1 ); +return -EPERM ; + } + + mcr =iosf_mbi_form_mcr (opcode, port, offset &MBI_MASK_LO ); + mcrx = offset &MBI_MASK_HI ; + +spin_lock_irqsave (&iosf_mbi_lock, flags); + ret =iosf_mbi_pci_write_fid (fid); +if (!ret) + ret =iosf_mbi_pci_write_mdr (mcrx, mcr, mdr); +spin_unlock_irqrestore (&iosf_mbi_lock, flags); + +return ret; +} +EXPORT_SYMBOL (iosf_mbi_write_with_fid); +bool
iosf_mbi_available(void)
{
/* Mbi isn't hot-pluggable. No remove routine is provided */
Signature · 2 lines
Andy Shevchenko <[email protected]> Intel Finland Oy