Hi, On 08-01-17 16:16, Andy Shevchenko wrote: > On Sun, 2017-01-08 at 14:44 +0100, Hans de Goede wrote: >> One some systems the punit accesses the pmic to change various >> voltages >> through the same bus as other kernel drivers use for e.g. battery >> monitoring. >> >> If a driver sends requests to the punit which require the punit to >> access >> the pmic bus while another driver is also accessing the pmic bus >> various >> bad things happen. >> >> This commit adds a mutex to protect the punit against simultaneous >> accesses >> and 2 functions to lock / unlock this mutex. >> >> Note on these systems the i2c-bus driver will request a sempahore from >> the >> punit for exclusive access to the pmic bus when i2c drivers are >> accessing >> it, but this does not appear to be sufficient, we still need to avoid >> making certain punit requests during the access window to avoid >> problems. > > I'm fine with the patch, but please spell > P-Unit > PMIC
In the commit msg and comments, not in code you mean I assume ? Regards, Hans > > Reviewed-by: Andy Shevchenko <andriy.shevchenko at linux.intel.com> > >> >> BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=155241 >> Signed-off-by: Hans de Goede <hdegoede at redhat.com> >> Tested-by: tagorereddy <tagore.chandan at gmail.com> >> --- >> arch/x86/include/asm/iosf_mbi.h | 31 >> +++++++++++++++++++++++++++++++ >> arch/x86/platform/intel/iosf_mbi.c | 13 +++++++++++++ >> 2 files changed, 44 insertions(+) >> >> diff --git a/arch/x86/include/asm/iosf_mbi.h >> b/arch/x86/include/asm/iosf_mbi.h >> index b41ee16..91f5d16 100644 >> --- a/arch/x86/include/asm/iosf_mbi.h >> +++ b/arch/x86/include/asm/iosf_mbi.h >> @@ -88,6 +88,33 @@ 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_punit_lock() - Lock the punit mutex >> + * >> + * One some systems the punit accesses the pmic to change various >> voltages >> + * through the same bus as other kernel drivers use for e.g. battery >> monitoring. >> + * >> + * If a driver sends requests to the punit which require the punit to >> access the >> + * pmic bus while another driver is also accessing the pmic bus >> various bad >> + * things happen. >> + * >> + * To avoid these problems this function must be called before >> accessing the >> + * punit or the pmic, be it through iosf_mbi* functions or through >> other means. >> + * >> + * Note on these systems the i2c-bus driver will request a sempahore >> from the >> + * punit for exclusive access to the pmic bus when i2c drivers are >> accessing it, >> + * but this does not appear to be sufficient, we still need to avoid >> making >> + * certain punit requests during the access window to avoid problems. >> + * >> + * This function locks a mutex, as such it may sleep. >> + */ >> +void iosf_mbi_punit_lock(void); >> + >> +/** >> + * iosf_mbi_punit_unlock() - Unlock the punit mutex >> + */ >> +void iosf_mbi_punit_unlock(void); >> + >> #else /* CONFIG_IOSF_MBI is not enabled */ >> static inline >> bool iosf_mbi_available(void) >> @@ -115,6 +142,10 @@ int iosf_mbi_modify(u8 port, u8 opcode, u32 >> offset, u32 mdr, u32 mask) >> WARN(1, "IOSF_MBI driver not available"); >> return -EPERM; >> } >> + >> +static inline void iosf_mbi_punit_lock(void) {} >> +static inline void iosf_mbi_punit_unlock(void) {} >> + >> #endif /* CONFIG_IOSF_MBI */ >> >> #endif /* IOSF_MBI_SYMS_H */ >> diff --git a/arch/x86/platform/intel/iosf_mbi.c >> b/arch/x86/platform/intel/iosf_mbi.c >> index edf2c54..75d8135 100644 >> --- a/arch/x86/platform/intel/iosf_mbi.c >> +++ b/arch/x86/platform/intel/iosf_mbi.c >> @@ -34,6 +34,7 @@ >> >> static struct pci_dev *mbi_pdev; >> static DEFINE_SPINLOCK(iosf_mbi_lock); >> +static DEFINE_MUTEX(iosf_mbi_punit_mutex); >> >> static inline u32 iosf_mbi_form_mcr(u8 op, u8 port, u8 offset) >> { >> @@ -190,6 +191,18 @@ bool iosf_mbi_available(void) >> } >> EXPORT_SYMBOL(iosf_mbi_available); >> >> +void iosf_mbi_punit_lock(void) >> +{ >> + mutex_lock(&iosf_mbi_punit_mutex); >> +} >> +EXPORT_SYMBOL(iosf_mbi_punit_lock); >> + >> +void iosf_mbi_punit_unlock(void) >> +{ >> + mutex_unlock(&iosf_mbi_punit_mutex); >> +} >> +EXPORT_SYMBOL(iosf_mbi_punit_unlock); >> + >> #ifdef CONFIG_IOSF_MBI_DEBUG >> static u32 dbg_mdr; >> static u32 dbg_mcr; >
