Re: [PATCH v8 08/10] tpm: Proxy driver for supporting multiple emulated TPMs
On Fri, Mar 18, 2016 at 10:52:00AM +0200, Jarkko Sakkinen wrote:
> On Thu, Mar 17, 2016 at 01:45:20PM -0400, Stefan Berger wrote:
> > On 03/16/2016 04:42 PM, Jarkko Sakkinen wrote:
> > >On Sun, Mar 13, 2016 at 06:54:38PM -0400, Stefan Berger wrote:
> > >>+
> > >>+/* above flags */
> > >>+#define VTPM_PROXY_FLAG_TPM2 1 /* emulator is TPM 2 */
> > >>+
> > >>+/* all supported flags */
> > >>+#define VTPM_PROXY_FLAGS_ALL (VTPM_PROXY_FLAG_TPM2)
> > >This can be moved inside the .c-file?
> >
> > I can move that.
> >
> > >
> > >>+
> > >>+#define VTPM_PROXY_MAGIC 0xa1
> > >>+
> > >>+#define VTPM_PROXY_IOC_NEW_DEV _IOW(VTPM_PROXY_MAGIC, 0x00, \
> > >>+ struct vtpm_proxy_new_dev)
> > >Could we simply replace these four lines with one line:
> > >
> > >#deifne VTPM_PROXY_IOC_NEW_DEV _IOW('t', 0x00, struct vtpm_proxy_new_dev);
> >
> > Does this make it better?
> >
> > >
> > >I changed the magic but does it matter?
> >
> > I would keep the magic at '0xa1'. The documentation is written to '0xa1' now
> > and seems to be good just as any other.
>
> OK. Works for me. Keep the ioctl definition as it is.
>
> Reviewed-by: Jarkko Sakkinen
>
> I can move the constant. You don't have to send a new patch version
> anymore. I start keeping this patch in my master but will not merge it
> to next before 4.6-rc5 so at the moment it would be scheduled for 4.7.
> Does this sound good for you?
>
> Further improvemnts should be sent as separate fixup patches.
Applied to git://git.infradead.org/users/jjs/linux-tpmdd.git.
/Jarkko
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH v4 7/7] powerpc/powernv/pci-ioda: Add IOMMU_CAP_INTR_REMAP for IODA host bridge
On Thu, 17 Mar 2016 19:38:29 +0800
Yongji Xie wrote:
> On 2016/3/17 0:32, Alex Williamson wrote:
> > On Mon, 7 Mar 2016 15:48:38 +0800
> > Yongji Xie wrote:
> >
> >> This patch adds IOMMU_CAP_INTR_REMAP for IODA host bridge so that
> >> we can mmap MSI-X table in vfio driver.
> >>
> >> Signed-off-by: Yongji Xie
> >> ---
> >> arch/powerpc/platforms/powernv/pci-ioda.c | 17 +
> >> 1 file changed, 17 insertions(+)
> >>
> >> diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c
> >> b/arch/powerpc/platforms/powernv/pci-ioda.c
> >> index f90dc04..f01b9ab 100644
> >> --- a/arch/powerpc/platforms/powernv/pci-ioda.c
> >> +++ b/arch/powerpc/platforms/powernv/pci-ioda.c
> >> @@ -1955,6 +1955,20 @@ static struct iommu_table_ops pnv_ioda2_iommu_ops =
> >> {
> >>.free = pnv_ioda2_table_free,
> >> };
> >>
> >> +static bool pnv_ioda_iommu_capable(enum iommu_cap cap)
> >> +{
> >> + switch (cap) {
> >> + case IOMMU_CAP_INTR_REMAP:
> >> + return true;
> >> + default:
> >> + return false;
> >> + }
> >> +}
> >> +
> >> +static struct iommu_ops pnv_ioda_iommu_ops = {
> >> + .capable = pnv_ioda_iommu_capable,
> >> +};
> >> +
> >> static void pnv_pci_ioda_setup_dma_pe(struct pnv_phb *phb,
> >> struct pnv_ioda_pe *pe, unsigned int base,
> >> unsigned int segs)
> >> @@ -3078,6 +3092,9 @@ static void pnv_pci_ioda_fixup(void)
> >>
> >>/* Link NPU IODA tables to their PCI devices. */
> >>pnv_npu_ioda_fixup();
> >> +
> >> + /* Add IOMMU_CAP_INTR_REMAP */
> >> + bus_set_iommu(&pci_bus_type, &pnv_ioda_iommu_ops);
> >> }
> >>
> >> /*
> >
> > Doesn't this set you up for a world of hurt? bus_set_iommu() calls
> > iommu_bus_init() which sets up notifiers, which maybe you don't care
> > about, but it also means that iommu_domain_alloc(&pci_bus_type) will
> > segfault because you're not providing a domain_alloc callback here.
>
> It seems to be hard to add IOMMU_CAP_INTR_REMAP on
> PPC64 platform.
>
> And can we add a new ioctl in vfio_iommu_driver to check
> if interrupt remapping is supported so that we can use our
> own way to determine that on PPC64 platform?
I'd prefer not. At the vfio user API level, the question is whether
the user can mmap over the msix table, testing a property/ioctl on the
iommu driver seems like an odd way to discover that. We should be
determining whether that's safe in the kernel and exporting that info
on the vfio device itself, where it seems like we have various ways we
could do this within the existing ioctls. Thanks,
Alex
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH v4 4/7] PCI: Modify resource_alignment to support multiple devices
On 2016/3/17 0:30, Alex Williamson wrote:
On Mon, 7 Mar 2016 15:48:35 +0800
Yongji Xie wrote:
When vfio passthrough a PCI device of which MMIO BARs
are smaller than PAGE_SIZE, guest will not handle the
mmio accesses to the BARs which leads to mmio emulations
in host.
This is because vfio will not allow to passthrough one
BAR's mmio page which may be shared with other BARs.
To solve this performance issue, this patch modifies
resource_alignment to support syntax where multiple
devices get the same alignment. So we can use something
like "pci=resource_alignment=*:*:*.*:noresize" to
enforce the alignment of all MMIO BARs to be at least
PAGE_SIZE so that one BAR's mmio page would not be
shared with other BARs.
Signed-off-by: Yongji Xie
---
Documentation/kernel-parameters.txt |2 +
drivers/pci/pci.c | 90 ++-
include/linux/pci.h |4 ++
3 files changed, 85 insertions(+), 11 deletions(-)
diff --git a/Documentation/kernel-parameters.txt
b/Documentation/kernel-parameters.txt
index 8028631..74b38ab 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -2918,6 +2918,8 @@ bytes respectively. Such letter suffixes can also be
entirely omitted.
aligned memory resources.
If is not specified,
PAGE_SIZE is used as alignment.
+ , , and can be set to
+ "*" which means match all values.
I don't see anywhere that you're automatically enabling this for your
platform, so presumably you're expecting users to determine on their
own that they have a performance problem and hoping that they'll figure
out that they need to use this option to resolve it. The first irate
question you'll get back is why doesn't this happen automatically?
Actually, I just want to make the code simple. Maybe it is
not a good idea to let users enable this option manually.
I will try to fix it like that:
diff --git a/arch/powerpc/include/asm/pci.h b/arch/powerpc/include/asm/pci.h
index 6f8065a..6659752 100644
--- a/arch/powerpc/include/asm/pci.h
+++ b/arch/powerpc/include/asm/pci.h
@@ -30,6 +30,8 @@
#define PCIBIOS_MIN_IO 0x1000
#define PCIBIOS_MIN_MEM0x1000
+#define PCIBIOS_MIN_ALIGNMENT PAGE_SIZE
+
struct pci_dev;
/* Values for the `which' argument to sys_pciconfig_iobase syscall. */
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index dadd28a..9f644e4 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -4593,6 +4593,8 @@ EXPORT_SYMBOL_GPL(pci_ignore_hotplug);
static char resource_alignment_param[RESOURCE_ALIGNMENT_PARAM_SIZE] = {0};
static DEFINE_SPINLOCK(resource_alignment_lock);
+#define DISABLE_ARCH_ALIGNMENT -1
+#define DEFAULT_ALIGNMENT -2
/**
* pci_specified_resource_alignment - get resource alignment specified
by user.
* @dev: the PCI device to get
@@ -4609,6 +4611,9 @@ static resource_size_t
pci_specified_resource_alignment(struct pci_dev *dev,
char *p;
bool invalid = false;
+#ifdef PCIBIOS_MIN_ALIGNMENT
+ align = PCIBIOS_MIN_ALIGNMENT;
+#endif
spin_lock(&resource_alignment_lock);
p = resource_alignment_param;
while (*p) {
@@ -4617,7 +4622,7 @@ static resource_size_t
pci_specified_resource_alignment(struct pci_dev *dev,
p[count] == '@') {
p += count + 1;
} else {
- align_order = -1;
+ align_order = DEFAULT_ALIGNMENT;
}
if (p[0] == '*' && p[1] == ':') {
seg = -1;
@@ -4673,8 +4678,10 @@ static resource_size_t
pci_specified_resource_alignment(struct pci_dev *dev,
(bus == dev->bus->number || bus == -1) &&
(slot == PCI_SLOT(dev->devfn) || slot == -1) &&
(func == PCI_FUNC(dev->devfn) || func == -1)) {
- if (align_order == -1)
+ if (align_order == DEFAULT_ALIGNMENT)
align = PAGE_SIZE;
+ else if (align_order == DISABLE_ARCH_ALIGNMENT)
+ align = 0;
else
align = 1 << align_order;
if (!pci_resources_page_aligned &&
PCI-PCI bridge can be specified, if resource
windows need to be expanded.
noresize: Don't change the resources' sizes when
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 760cce5..44ab59f 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -102,6 +102,8 @@ unsigned int pcibios_max_latency = 255;
/* If set, the PCIe ARI capability will not be used. */
static bool
[PATCH v5 3/6] module: s390: keep mod_arch_specific for livepatch modules
Livepatch needs to utilize the symbol information contained in the
mod_arch_specific struct in order to be able to call the s390
apply_relocate_add() function to apply relocations. Keep a reference to
syminfo if the module is a livepatch module. Remove the redundant vfree()
in module_finalize() since module_arch_freeing_init() (which also frees
those structures) is called in do_init_module(). If the module isn't a
livepatch module, we free the structures in module_arch_freeing_init() as
usual.
Signed-off-by: Jessica Yu
---
arch/s390/kernel/module.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/arch/s390/kernel/module.c b/arch/s390/kernel/module.c
index 7873e17..fbc0789 100644
--- a/arch/s390/kernel/module.c
+++ b/arch/s390/kernel/module.c
@@ -51,6 +51,10 @@ void *module_alloc(unsigned long size)
void module_arch_freeing_init(struct module *mod)
{
+ if (is_livepatch_module(mod) &&
+ mod->state == MODULE_STATE_LIVE)
+ return;
+
vfree(mod->arch.syminfo);
mod->arch.syminfo = NULL;
}
@@ -425,7 +429,5 @@ int module_finalize(const Elf_Ehdr *hdr,
struct module *me)
{
jump_label_apply_nops(me);
- vfree(me->arch.syminfo);
- me->arch.syminfo = NULL;
return 0;
}
--
2.4.3
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH v4 0/7] vfio-pci: Allow to mmap sub-page MMIO BARs and MSI-X table
On 2016/3/16 22:10, Bjorn Helgaas wrote: On Wed, Mar 16, 2016 at 06:51:56PM +0800, Yongji Xie wrote: Ping. This is mainly VFIO stuff, and Alex had some security concerns, so I'm not going to spend much time looking at this until he's satisfied. When I do, I'll be looking hard at the resource_alignment kernel parameter. I'm opposed to kernel parameters in general because they're very difficult for users to use correctly, and they lead to kernel code paths that are rarely tested and hard to maintain. So I'll be looking for an excuse to reject changes in that area. The changelog for 2/7 says it "replaces IORESOURCE_STARTALIGN with IORESOURCE_WINDOW." But even a glance at the patch itself shows that IORESOURCE_WINDOW is *added* to some places, and it doesn't *replace* IORESOURCE_STARTALIGN. There is a problem with my statement. I mean we can use IORESOURCE_WINDOW to identify bridge resources instead of IORESOURCE_STARTALIGN here. The changelog for 4/7 says: This is because vfio will not allow to passthrough one BAR's mmio page which may be shared with other BARs. To solve this performance issue ... with no mention at all of the actual *reason* vfio doesn't allow that passthrough. If I understand correctly, that reason has to do with security, so your justification must be much stronger than "solving a performance issue." OK. I will try to make my justification become stronger. Thanks, Yongji Xie -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH v4 7/7] powerpc/powernv/pci-ioda: Add IOMMU_CAP_INTR_REMAP for IODA host bridge
On 2016/3/17 20:48, Alex Williamson wrote:
On Thu, 17 Mar 2016 19:38:29 +0800
Yongji Xie wrote:
On 2016/3/17 0:32, Alex Williamson wrote:
On Mon, 7 Mar 2016 15:48:38 +0800
Yongji Xie wrote:
This patch adds IOMMU_CAP_INTR_REMAP for IODA host bridge so that
we can mmap MSI-X table in vfio driver.
Signed-off-by: Yongji Xie
---
arch/powerpc/platforms/powernv/pci-ioda.c | 17 +
1 file changed, 17 insertions(+)
diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c
b/arch/powerpc/platforms/powernv/pci-ioda.c
index f90dc04..f01b9ab 100644
--- a/arch/powerpc/platforms/powernv/pci-ioda.c
+++ b/arch/powerpc/platforms/powernv/pci-ioda.c
@@ -1955,6 +1955,20 @@ static struct iommu_table_ops pnv_ioda2_iommu_ops = {
.free = pnv_ioda2_table_free,
};
+static bool pnv_ioda_iommu_capable(enum iommu_cap cap)
+{
+ switch (cap) {
+ case IOMMU_CAP_INTR_REMAP:
+ return true;
+ default:
+ return false;
+ }
+}
+
+static struct iommu_ops pnv_ioda_iommu_ops = {
+ .capable = pnv_ioda_iommu_capable,
+};
+
static void pnv_pci_ioda_setup_dma_pe(struct pnv_phb *phb,
struct pnv_ioda_pe *pe, unsigned int base,
unsigned int segs)
@@ -3078,6 +3092,9 @@ static void pnv_pci_ioda_fixup(void)
/* Link NPU IODA tables to their PCI devices. */
pnv_npu_ioda_fixup();
+
+ /* Add IOMMU_CAP_INTR_REMAP */
+ bus_set_iommu(&pci_bus_type, &pnv_ioda_iommu_ops);
}
/*
Doesn't this set you up for a world of hurt? bus_set_iommu() calls
iommu_bus_init() which sets up notifiers, which maybe you don't care
about, but it also means that iommu_domain_alloc(&pci_bus_type) will
segfault because you're not providing a domain_alloc callback here.
It seems to be hard to add IOMMU_CAP_INTR_REMAP on
PPC64 platform.
And can we add a new ioctl in vfio_iommu_driver to check
if interrupt remapping is supported so that we can use our
own way to determine that on PPC64 platform?
I'd prefer not. At the vfio user API level, the question is whether
the user can mmap over the msix table, testing a property/ioctl on the
iommu driver seems like an odd way to discover that. We should be
determining whether that's safe in the kernel and exporting that info
on the vfio device itself, where it seems like we have various ways we
could do this within the existing ioctls. Thanks,
Alex
Yes, you are right. It's not a good idea to add a new ioctl in
vfio_iommu_driver. Now I'd like to talk about the way to
determining whether it's safe to mmap over the msix table.
We currently use IOMMU_CAP_INTR_REMAP to determine that.
But there are some problems on PPC64 which never set
iommu_ops and ARM SMMU which set this capability but not
provide interrupt isolation. Can we add a variable/property
which can be set in vfio_iommu_driver->ops->attach_group()
and used in vfio_pci_driver to determine whether we can allow
mmapping msix table? If so, we can still use
IOMMU_CAP_INTR_REMAP, or some arch-independent ways
when IOMMU_CAP_INTR_REMAP doesn't work.
Thanks,
Yongji Xie
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/8] clk: bcm2835: add missing osc and per clocks
ker...@martin.sperl.org writes: > From: Martin Sperl > > Add definitions for the following clocks: > * AVE0 > * DFT > * GP0 > * GP1 > * GP2 > * PULSE > * SLIM > * SMI > * TEC > > Signed-off-by: Martin Sperl > --- > + [BCM2835_CLOCK_DFT] = REGISTER_PER_CLK( > + .name = "dft", > + .ctl_reg = CM_DFTCTL, > + .div_reg = CM_DFTDIV, > + .int_bits = 5, > + .frac_bits = 0), > + [BCM2835_CLOCK_DFT] = REGISTER_PER_CLK( > + .name = "dpi", > + .ctl_reg = CM_DPICTL, > + .div_reg = CM_DPIDIV, > + .int_bits = 4, > + .frac_bits = 8), Oh, I see you've also doubled up the DFT definition here, when it seems you meant to define DPI. I've added a DPI enum value to fix it. signature.asc Description: PGP signature
Re: [PATCH 3/9] serial: doc: Document .throttle()
On Mon, Mar 14, 2016 at 4:16 PM, Geert Uytterhoeven
wrote:
> Signed-off-by: Geert Uytterhoeven
> ---
> Documentation/serial/driver | 7 +++
> 1 file changed, 7 insertions(+)
>
> diff --git a/Documentation/serial/driver b/Documentation/serial/driver
> index 61d520dea4c6e13a..50f3d94ed50b341e 100644
> --- a/Documentation/serial/driver
> +++ b/Documentation/serial/driver
> @@ -126,6 +126,13 @@ hardware.
> Interrupts: locally disabled.
> This call must not sleep
>
> + throttle(port)
> + Notify the serial driver that input buffers for the line discipline
> are
> + close to full, and it should somehow signal that no more characters
> + should be sent to the serial port.
> +
> + Locking: none.
> +
In the mean time I discovered this (and .unthrottle()) is used with hardware
assisted flow control only, and when it was introduced.
Will update the documentation accordingly.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH v4 6/7] vfio-pci: Allow to mmap MSI-X table if IOMMU_CAP_INTR_REMAP was set
On 2016/3/17 0:31, Alex Williamson wrote:
[cc+ Eric, Will]
On Mon, 7 Mar 2016 15:48:37 +0800
Yongji Xie wrote:
Current vfio-pci implementation disallows to mmap MSI-X
table in case that user get to touch this directly.
But we should allow to mmap these MSI-X tables if IOMMU
supports interrupt remapping which can ensure that a
given pci device can only shoot the MSIs assigned for it.
Signed-off-by: Yongji Xie
---
drivers/vfio/pci/vfio_pci.c |8 +---
drivers/vfio/pci/vfio_pci_rdwr.c |4 +++-
2 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
index 49d7a69..d6f4788 100644
--- a/drivers/vfio/pci/vfio_pci.c
+++ b/drivers/vfio/pci/vfio_pci.c
@@ -592,13 +592,14 @@ static long vfio_pci_ioctl(void *device_data,
IORESOURCE_MEM && !pci_resources_share_page(pdev,
info.index)) {
info.flags |= VFIO_REGION_INFO_FLAG_MMAP;
- if (info.index == vdev->msix_bar) {
+ if (!iommu_capable(pdev->dev.bus,
+ IOMMU_CAP_INTR_REMAP) &&
+ info.index == vdev->msix_bar) {
We only need to test the IOMMU capability if it's the msix BAR, so why
test these in the reverse order? It should be:
info.index == vdev->msix_bar &&
!iommu_capable(pdev->dev.bus,
IOMMU_CAP_INTR_REMAP)
Same below.
OK. I'll fix it.
I think we also have the problem that ARM SMMU is setting this
capability when it's really not doing anything at all to provide
interrupt isolation. Adding Eric and Will to the Cc for comment.
I slightly dislike using an IOMMU API interface here to determine if
it's safe to allow user access to the MSIx vector table, but it seems
like the best option we have at this point, if it's actually true for
all the IOMMU drivers participating in the IOMMU API.
ret = msix_sparse_mmap_cap(vdev, &caps);
if (ret)
return ret;
}
}
-
break;
case VFIO_PCI_ROM_REGION_INDEX:
{
@@ -1029,7 +1030,8 @@ static int vfio_pci_mmap(void *device_data, struct
vm_area_struct *vma)
if (phys_len < PAGE_SIZE || req_start + req_len > phys_len)
return -EINVAL;
- if (index == vdev->msix_bar) {
+ if (!iommu_capable(pdev->dev.bus, IOMMU_CAP_INTR_REMAP) &&
+ index == vdev->msix_bar) {
/*
* Disallow mmaps overlapping the MSI-X table; users don't
* get to touch this directly. We could find somewhere
diff --git a/drivers/vfio/pci/vfio_pci_rdwr.c b/drivers/vfio/pci/vfio_pci_rdwr.c
index 5ffd1d9..1c46c29 100644
--- a/drivers/vfio/pci/vfio_pci_rdwr.c
+++ b/drivers/vfio/pci/vfio_pci_rdwr.c
@@ -18,6 +18,7 @@
#include
#include
#include
+#include
#include "vfio_pci_private.h"
@@ -164,7 +165,8 @@ ssize_t vfio_pci_bar_rw(struct vfio_pci_device *vdev, char __user *buf,
} else
io = vdev->barmap[bar];
- if (bar == vdev->msix_bar) {
+ if (!iommu_capable(pdev->dev.bus, IOMMU_CAP_INTR_REMAP) &&
+ bar == vdev->msix_bar) {
Do we really want to test this on *every* read/write to any BAR (order
of tests matter)? Even in the case of the MSIx BAR, should we cache
this when the device is first opened?
I will cache this in vfio_pci_open().
Thanks,
Yongji Xie
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v6 1/4] i2c: add a protocol parameter to the alert callback
.alert() is meant to be generic, but there is currently no way
for the device driver to know which protocol generated the alert.
Add a parameter in .alert() to help the device driver to understand
what is given in data.
This patch is required to have the support of SMBus Host Notify protocol
through .alert().
Signed-off-by: Benjamin Tissoires
---
new in v2
changes in v3:
- added also lm90.c to support the new API
no changes in v4
no changes in v5
changes in v6:
- made sure lm90 also checks for the type of alert first
drivers/char/ipmi/ipmi_ssif.c | 6 +-
drivers/hwmon/lm90.c | 6 +-
drivers/i2c/i2c-smbus.c | 3 ++-
include/linux/i2c.h | 7 ++-
4 files changed, 18 insertions(+), 4 deletions(-)
diff --git a/drivers/char/ipmi/ipmi_ssif.c b/drivers/char/ipmi/ipmi_ssif.c
index 5f1c3d0..1d67fa3 100644
--- a/drivers/char/ipmi/ipmi_ssif.c
+++ b/drivers/char/ipmi/ipmi_ssif.c
@@ -568,12 +568,16 @@ static void retry_timeout(unsigned long data)
}
-static void ssif_alert(struct i2c_client *client, unsigned int data)
+static void ssif_alert(struct i2c_client *client, enum i2c_alert_protocol type,
+ unsigned int data)
{
struct ssif_info *ssif_info = i2c_get_clientdata(client);
unsigned long oflags, *flags;
bool do_get = false;
+ if (type != I2C_PROTOCOL_SMBUS_ALERT)
+ return;
+
ssif_inc_stat(ssif_info, alerts);
flags = ipmi_ssif_lock_cond(ssif_info, &oflags);
diff --git a/drivers/hwmon/lm90.c b/drivers/hwmon/lm90.c
index c9ff08d..a00fd38 100644
--- a/drivers/hwmon/lm90.c
+++ b/drivers/hwmon/lm90.c
@@ -1624,10 +1624,14 @@ static int lm90_remove(struct i2c_client *client)
return 0;
}
-static void lm90_alert(struct i2c_client *client, unsigned int flag)
+static void lm90_alert(struct i2c_client *client, enum i2c_alert_protocol type,
+ unsigned int flag)
{
u16 alarms;
+ if (type != I2C_PROTOCOL_SMBUS_ALERT)
+ return;
+
if (lm90_is_tripped(client, &alarms)) {
/*
* Disable ALERT# output, because these chips don't implement
diff --git a/drivers/i2c/i2c-smbus.c b/drivers/i2c/i2c-smbus.c
index abb55d3..3b6765a 100644
--- a/drivers/i2c/i2c-smbus.c
+++ b/drivers/i2c/i2c-smbus.c
@@ -56,7 +56,8 @@ static int smbus_do_alert(struct device *dev, void *addrp)
if (client->dev.driver) {
driver = to_i2c_driver(client->dev.driver);
if (driver->alert)
- driver->alert(client, data->flag);
+ driver->alert(client, I2C_PROTOCOL_SMBUS_ALERT,
+ data->flag);
else
dev_warn(&client->dev, "no driver alert()!\n");
} else
diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index 200cf13b..baae02a 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -126,6 +126,10 @@ i2c_smbus_read_i2c_block_data_or_emulated(const struct
i2c_client *client,
u8 command, u8 length, u8 *values);
#endif /* I2C */
+enum i2c_alert_protocol {
+ I2C_PROTOCOL_SMBUS_ALERT,
+};
+
/**
* struct i2c_driver - represent an I2C device driver
* @class: What kind of i2c device we instantiate (for detect)
@@ -181,7 +185,8 @@ struct i2c_driver {
* For the SMBus alert protocol, there is a single bit of data passed
* as the alert response's low bit ("event flag").
*/
- void (*alert)(struct i2c_client *, unsigned int data);
+ void (*alert)(struct i2c_client *, enum i2c_alert_protocol protocol,
+ unsigned int data);
/* a ioctl like command that can be used to perform specific functions
* with the device.
--
2.5.0
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v6 2/4] i2c-smbus: add SMBus Host Notify support
SMBus Host Notify allows a slave device to act as a master on a bus to
notify the host of an interrupt. On Intel chipsets, the functionality
is directly implemented in the firmware. We just need to export a
function to call .alert() on the proper device driver.
i2c_handle_smbus_host_notify() behaves like i2c_handle_smbus_alert().
When called, it schedules a task that will be able to sleep to go through
the list of devices attached to the adapter.
The current implementation allows one Host Notification to be scheduled
while an other is running.
Signed-off-by: Benjamin Tissoires
---
changes in v2:
- do the actual processing of finding the device in i2c-smbus.c
- remove the i2c-core implementations
- remove the manual toggle of SMBus Host Notify
no changes in v3
changes in v4:
- schedule the worker in i2c_handle_smbus_host_notify() -> it can now be called
in an interrupt context.
- introduce i2c_setup_smbus_host_notify()
no changes in v5
changes in v6:
- fix the parameters of the inlined functions when the module is not compiled
Documentation/i2c/smbus-protocol | 3 ++
drivers/i2c/i2c-smbus.c | 113 +--
include/linux/i2c-smbus.h| 44 +++
include/linux/i2c.h | 3 ++
include/uapi/linux/i2c.h | 1 +
5 files changed, 159 insertions(+), 5 deletions(-)
diff --git a/Documentation/i2c/smbus-protocol b/Documentation/i2c/smbus-protocol
index 6012b12..4e07848 100644
--- a/Documentation/i2c/smbus-protocol
+++ b/Documentation/i2c/smbus-protocol
@@ -199,6 +199,9 @@ alerting device's address.
[S] [HostAddr] [Wr] A [DevAddr] A [DataLow] A [DataHigh] A [P]
+I2C drivers for devices which can trigger SMBus Host Notify should implement
+the optional alert() callback.
+
Packet Error Checking (PEC)
===
diff --git a/drivers/i2c/i2c-smbus.c b/drivers/i2c/i2c-smbus.c
index 3b6765a..1de7ee5 100644
--- a/drivers/i2c/i2c-smbus.c
+++ b/drivers/i2c/i2c-smbus.c
@@ -33,7 +33,8 @@ struct i2c_smbus_alert {
struct alert_data {
unsigned short addr;
- u8 flag:1;
+ enum i2c_alert_protocol type;
+ unsigned intdata;
};
/* If this is the alerting device, notify its driver */
@@ -56,8 +57,7 @@ static int smbus_do_alert(struct device *dev, void *addrp)
if (client->dev.driver) {
driver = to_i2c_driver(client->dev.driver);
if (driver->alert)
- driver->alert(client, I2C_PROTOCOL_SMBUS_ALERT,
- data->flag);
+ driver->alert(client, data->type, data->data);
else
dev_warn(&client->dev, "no driver alert()!\n");
} else
@@ -97,8 +97,9 @@ static void smbus_alert(struct work_struct *work)
if (status < 0)
break;
- data.flag = status & 1;
+ data.data = status & 1;
data.addr = status >> 1;
+ data.type = I2C_PROTOCOL_SMBUS_ALERT;
if (data.addr == prev_addr) {
dev_warn(&ara->dev, "Duplicate SMBALERT# from dev "
@@ -106,7 +107,7 @@ static void smbus_alert(struct work_struct *work)
break;
}
dev_dbg(&ara->dev, "SMBALERT# from dev 0x%02x, flag %d\n",
- data.addr, data.flag);
+ data.addr, data.data);
/* Notify driver for the device which issued the alert */
device_for_each_child(&ara->adapter->dev, &data,
@@ -240,6 +241,108 @@ int i2c_handle_smbus_alert(struct i2c_client *ara)
}
EXPORT_SYMBOL_GPL(i2c_handle_smbus_alert);
+static void smbus_host_notify_work(struct work_struct *work)
+{
+ struct alert_data alert;
+ struct i2c_adapter *adapter;
+ unsigned long flags;
+ u16 payload;
+ u8 addr;
+ struct smbus_host_notify *data;
+
+ data = container_of(work, struct smbus_host_notify, work);
+
+ spin_lock_irqsave(&data->lock, flags);
+ payload = data->payload;
+ addr = data->addr;
+ adapter = data->adapter;
+
+ /* clear the pending bit and release the spinlock */
+ data->pending = false;
+ spin_unlock_irqrestore(&data->lock, flags);
+
+ if (!adapter || !addr)
+ return;
+
+ alert.type = I2C_PROTOCOL_SMBUS_HOST_NOTIFY;
+ alert.addr = addr;
+ alert.data = payload;
+
+ device_for_each_child(&adapter->dev, &alert, smbus_do_alert);
+}
+
+/**
+ * i2c_setup_smbus_host_notify - Allocate a new smbus_host_notify for the given
+ * I2C adapter.
+ * @adapter: the adapter we want to associate a Host Notify function
+ *
+ * Returns a struct smbus_host_notify pointer on success, and NULL on failure.
+ * The resulting smbus_host_notify must not be freed afterwards, it is a
+ * managed resource already.
+ */
+struct smbus_
Re: [PATCH v6 1/4] i2c: add a protocol parameter to the alert callback
On Wed, Mar 16, 2016 at 05:39:05PM +0100, Benjamin Tissoires wrote:
> .alert() is meant to be generic, but there is currently no way
> for the device driver to know which protocol generated the alert.
> Add a parameter in .alert() to help the device driver to understand
> what is given in data.
>
> This patch is required to have the support of SMBus Host Notify protocol
> through .alert().
>
> Signed-off-by: Benjamin Tissoires
For hwmon:
Acked-by: Guenter Roeck
> ---
>
> new in v2
>
> changes in v3:
> - added also lm90.c to support the new API
>
> no changes in v4
>
> no changes in v5
>
> changes in v6:
> - made sure lm90 also checks for the type of alert first
>
> drivers/char/ipmi/ipmi_ssif.c | 6 +-
> drivers/hwmon/lm90.c | 6 +-
> drivers/i2c/i2c-smbus.c | 3 ++-
> include/linux/i2c.h | 7 ++-
> 4 files changed, 18 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/char/ipmi/ipmi_ssif.c b/drivers/char/ipmi/ipmi_ssif.c
> index 5f1c3d0..1d67fa3 100644
> --- a/drivers/char/ipmi/ipmi_ssif.c
> +++ b/drivers/char/ipmi/ipmi_ssif.c
> @@ -568,12 +568,16 @@ static void retry_timeout(unsigned long data)
> }
>
>
> -static void ssif_alert(struct i2c_client *client, unsigned int data)
> +static void ssif_alert(struct i2c_client *client, enum i2c_alert_protocol
> type,
> +unsigned int data)
> {
> struct ssif_info *ssif_info = i2c_get_clientdata(client);
> unsigned long oflags, *flags;
> bool do_get = false;
>
> + if (type != I2C_PROTOCOL_SMBUS_ALERT)
> + return;
> +
> ssif_inc_stat(ssif_info, alerts);
>
> flags = ipmi_ssif_lock_cond(ssif_info, &oflags);
> diff --git a/drivers/hwmon/lm90.c b/drivers/hwmon/lm90.c
> index c9ff08d..a00fd38 100644
> --- a/drivers/hwmon/lm90.c
> +++ b/drivers/hwmon/lm90.c
> @@ -1624,10 +1624,14 @@ static int lm90_remove(struct i2c_client *client)
> return 0;
> }
>
> -static void lm90_alert(struct i2c_client *client, unsigned int flag)
> +static void lm90_alert(struct i2c_client *client, enum i2c_alert_protocol
> type,
> +unsigned int flag)
> {
> u16 alarms;
>
> + if (type != I2C_PROTOCOL_SMBUS_ALERT)
> + return;
> +
> if (lm90_is_tripped(client, &alarms)) {
> /*
>* Disable ALERT# output, because these chips don't implement
> diff --git a/drivers/i2c/i2c-smbus.c b/drivers/i2c/i2c-smbus.c
> index abb55d3..3b6765a 100644
> --- a/drivers/i2c/i2c-smbus.c
> +++ b/drivers/i2c/i2c-smbus.c
> @@ -56,7 +56,8 @@ static int smbus_do_alert(struct device *dev, void *addrp)
> if (client->dev.driver) {
> driver = to_i2c_driver(client->dev.driver);
> if (driver->alert)
> - driver->alert(client, data->flag);
> + driver->alert(client, I2C_PROTOCOL_SMBUS_ALERT,
> + data->flag);
> else
> dev_warn(&client->dev, "no driver alert()!\n");
> } else
> diff --git a/include/linux/i2c.h b/include/linux/i2c.h
> index 200cf13b..baae02a 100644
> --- a/include/linux/i2c.h
> +++ b/include/linux/i2c.h
> @@ -126,6 +126,10 @@ i2c_smbus_read_i2c_block_data_or_emulated(const struct
> i2c_client *client,
> u8 command, u8 length, u8 *values);
> #endif /* I2C */
>
> +enum i2c_alert_protocol {
> + I2C_PROTOCOL_SMBUS_ALERT,
> +};
> +
> /**
> * struct i2c_driver - represent an I2C device driver
> * @class: What kind of i2c device we instantiate (for detect)
> @@ -181,7 +185,8 @@ struct i2c_driver {
>* For the SMBus alert protocol, there is a single bit of data passed
>* as the alert response's low bit ("event flag").
>*/
> - void (*alert)(struct i2c_client *, unsigned int data);
> + void (*alert)(struct i2c_client *, enum i2c_alert_protocol protocol,
> + unsigned int data);
>
> /* a ioctl like command that can be used to perform specific functions
>* with the device.
> --
> 2.5.0
>
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
