| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <002401d7f016$3bb4f260$b31ed720$@opensource.cirrus.com>
Date: Mon, 13 Dec 2021 11:40:30 +0000
From: Stefan Binding <sbinding@...nsource.cirrus.com>
To: 'Hans de Goede' <hdegoede@...hat.com>,
'Mark Brown' <broonie@...nel.org>,
"'Rafael J . Wysocki'" <rafael@...nel.org>,
'Len Brown' <lenb@...nel.org>,
'Mark Gross' <markgross@...nel.org>
CC: <linux-kernel@...r.kernel.org>, <linux-spi@...r.kernel.org>,
<linux-acpi@...r.kernel.org>,
<platform-driver-x86@...r.kernel.org>,
<patches@...nsource.cirrus.com>
Subject: RE: [PATCH v2 0/6] Support Spi in i2c-multi-instantiate driver
Hi,
> -----Original Message-----
> From: Hans de Goede <hdegoede@...hat.com>
> Sent: 10 December 2021 16:55
> To: Stefan Binding <sbinding@...nsource.cirrus.com>; Mark Brown
> <broonie@...nel.org>; Rafael J . Wysocki <rafael@...nel.org>; Len Brown
> <lenb@...nel.org>; Mark Gross <markgross@...nel.org>
> Cc: linux-kernel@...r.kernel.org; linux-spi@...r.kernel.org; linux-
> acpi@...r.kernel.org; platform-driver-x86@...r.kernel.org;
> patches@...nsource.cirrus.com
> Subject: Re: [PATCH v2 0/6] Support Spi in i2c-multi-instantiate driver
>
> Hi Stefan,
>
> On 12/10/21 16:40, Stefan Binding wrote:
> > Add support for SPI bus in the ic2-multi-instantiate driver as
> > upcoming laptops will need to multi instantiate SPI devices from a
> > single device node, which has multiple SpiSerialBus entries at the
> > ACPI table.
> >
> > With the new SPI support, i2c-multi-instantiate becomes
> > bus-multi-instantiate and is moved to the ACPI folder.
> >
> > The intention is to support the SPI bus by re-using the current I2C
> > multi instantiate, instead of creating a new SPI multi instantiate, to
> > make it possible for peripherals that can be controlled by I2C or SPI
> > to have the same HID at the ACPI table.
> >
> > The new driver (Bus multi instantiate, bmi) checks for the hard-coded
> > bus type and returns -ENODEV in case of zero devices found for that
> > bus. In the case of automatic bus detection, the driver will give
> > preference to I2C.
> >
> > The expectation is for a device node in the ACPI table to have
> > multiple I2cSerialBus only or multiple SpiSerialBus only, not a mix of
> > both; and for the case where there are both entries in one device
> > node, only the I2C ones would be probed.
> >
> > This new bus multi instantiate will be used in CS35L41 HDA new driver,
> > being upstreamed:
> > https://lkml.org/lkml/2021/11/23/723
>
> Unfortunately you never really answered my questions about v1 of this
> series:
>
> https://lore.kernel.org/platform-driver-x86/a1f546c2-5c63-573a-c032-
> 603c792f3f7c@...hat.com/
>
> So looking at the linked CS35L41 HDA series there is a single ACPI device node
> with a HID of CLSA0100 which describes two CS35L41 amplifiers connected
> over I2C ?
Yes, the related series uses HID CLSA0100, which contains 2 I2C devices inside a
single node. This ID was mistakenly used for this laptop, and instead CSC3551
has been used for subsequent laptops.
>
> I assume you are doing this work because there are also designs where there
> is a similar CLSA0100 ACPI device which also describes two CS35L41 amplifiers
> but then connected over SPI ?
Yes, there are several laptop designs which use an equivalent ACPI which describes
2 or 4 CS35L41 amplifiers which are connected either via I2C or via SPI.
Both designs use the same ACPI design and have 2-4 devices (either I2C or SPI)
defined inside a single ACPI node for HID CSC3551.
Note that the devices inside the node can be either SPI or I2C, but never SPI
and I2C.
>
> It would really help if you can:
>
> 1. Answer my questions from v1
I hope my colleague Lucas has answered these questions now.
> 2. Provide a concrete example of a device where these changes will be
> necessary to make things work, preferably with a link to an actual ACPI DSDT
> of that device.
This is the CSC3551 node for a laptop with 4 SPI nodes, with a shared IRQ:
Scope (_SB.PC00.SPI0)
{
Device (GSPK)
{
Name (_HID, "CSC3551") // _HID: Hardware ID
Method (AUID, 0, NotSerialized)
{
Return ("103C89C3")
}
Method (_SUB, 0, NotSerialized) // _SUB: Subsystem ID
{
Return (AUID ())
}
Name (_UID, One) // _UID: Unique ID
Method (_CRS, 0, NotSerialized) // _CRS: Current Resource Settings
{
Name (RBUF, ResourceTemplate ()
{
SpiSerialBusV2 (0x0000, PolarityLow, FourWireMode, 0x08,
ControllerInitiated, 0x003D0900, ClockPolarityLow,
ClockPhaseFirst, "\\_SB.PC00.SPI0",
0x00, ResourceConsumer, , Exclusive,
)
SpiSerialBusV2 (0x0001, PolarityLow, FourWireMode, 0x08,
ControllerInitiated, 0x003D0900, ClockPolarityLow,
ClockPhaseFirst, "\\_SB.PC00.SPI0",
0x00, ResourceConsumer, , Exclusive,
)
SpiSerialBusV2 (0x0002, PolarityLow, FourWireMode, 0x08,
ControllerInitiated, 0x003D0900, ClockPolarityLow,
ClockPhaseFirst, "\\_SB.PC00.SPI0",
0x00, ResourceConsumer, , Exclusive,
)
SpiSerialBusV2 (0x0003, PolarityLow, FourWireMode, 0x08,
ControllerInitiated, 0x003D0900, ClockPolarityLow,
ClockPhaseFirst, "\\_SB.PC00.SPI0",
0x00, ResourceConsumer, , Exclusive,
)
GpioIo (Exclusive, PullUp, 0x0000, 0x0000, IoRestrictionOutputOnly,
"\\_SB.GPI0", 0x00, ResourceConsumer, ,
)
{ // Pin list
0x0000
}
GpioIo (Exclusive, PullUp, 0x0000, 0x0000, IoRestrictionOutputOnly,
"\\_SB.GPI0", 0x00, ResourceConsumer, ,
)
{ // Pin list
0x0000
}
GpioIo (Exclusive, PullUp, 0x0000, 0x0000, IoRestrictionOutputOnly,
"\\_SB.GPI0", 0x00, ResourceConsumer, ,
)
{ // Pin list
0x0000
}
GpioIo (Exclusive, PullDown, 0x0000, 0x0000, IoRestrictionOutputOnly,
"\\_SB.GPI0", 0x00, ResourceConsumer, ,
)
{ // Pin list
0x0000
}
GpioIo (Shared, PullUp, 0x0064, 0x0000, IoRestrictionInputOnly,
"\\_SB.GPI0", 0x00, ResourceConsumer, ,
)
{ // Pin list
0x0000
}
GpioInt (Edge, ActiveBoth, Shared, PullUp, 0x0064,
"\\_SB.GPI0", 0x00, ResourceConsumer, ,
)
{ // Pin list
0x0000
}
})
CreateWordField (RBUF, 0xCA, ACS1)
CreateWordField (RBUF, 0xA7, ACS2)
CreateWordField (RBUF, 0xED, ACS3)
CreateWordField (RBUF, 0x0110, ARST)
CreateWordField (RBUF, 0x0133, AINT)
CreateWordField (RBUF, 0x0156, AIN2)
ACS1 = GNUM (0x090E0016)
ACS2 = GNUM (0x090E0017)
ACS3 = GNUM (0x090C0006)
ARST = GNUM (0x09070017)
AINT = GNUM (0x09070013)
AIN2 = GNUM (0x09070013)
Return (RBUF) /* \_SB_.PC00.SPI0.GSPK._CRS.RBUF */
}
Method (_STA, 0, NotSerialized) // _STA: Status
{
Return (0x0F)
}
Method (_DIS, 0, NotSerialized) // _DIS: Disable Device
{
}
Name (_DSD, Package () // _DSD: Device-Specific Data
{
ToUUID ("daffd814-6eba-4d8c-8a91-bc9bbf4aa301") /* Device Properties for _DSD */,
Package ()
{
Package () { "cirrus,dev-index", Package () { 0, 1, 2, 3 } },
Package ()
{
"reset-gpios", Package ()
{
^GSPK, 3, 0, 0,
^GSPK, 3, 0, 0,
^GSPK, 3, 0, 0,
^GSPK, 3, 0, 0,
},
},
Package () { "cirrus,speaker-position", Package () { 1, 0, 1, 0 } },
Package () { "cirrus,gpio1-func", Package () { 3, 3, 3, 3 } },
Package () { "cirrus,gpio2-func", Package () { 2, 2, 2, 2 } },
Package () { "cirrus,boost-ind-nanohenry", Package () { 1000, 1000, 1000, 1000 } },
Package () { "cirrus,boost-peak-milliamp", Package () { 4500, 4500, 4500, 4500 } },
Package () { "cirrus,boost-cap-microfarad", Package () { 24, 24, 24, 24 } },
}
})
}
}
This is just our node from the DSDT, we are working to obtain and share the full DSDT, if still required.
>
> Until you can better clarify why this is necessary, this series gets a nack from
> me. The i2c-mult-instantiate code is a hack to deal with some rather sub-
> optimal choices made in DSDTs used on devices shipped with Windows and
> unless absolutely necessary I would rather not see this get expanded to SPI.
>
> Regards,
>
> Hans
Thanks,
Stefan
Powered by blists - more mailing lists