On 2/1/21 10:39 PM, Vinod Koul wrote:
On 01-02-21, 10:18, Pierre-Louis Bossart wrote:
On 2/1/21 4:38 AM, Vinod Koul wrote:
On 01-02-21, 15:58, Vinod Koul wrote:
On 26-01-21, 16:37, Bard Liao wrote:
struct sdw_master_prop {
u32 revision;
@@ -421,8 +422,11 @@ struct sdw_master_prop {
u32 err_threshold;
u32 mclk_freq;
bool hw_disabled;
+ u32 quirks;
Can we do u64 here please.. I dont know where we would end up.. but
would hate if we start running out of space ..
No objection.
Also, is the sdw_master_prop right place for a 'quirk' property. I think
we can use sdw_master_device or sdw_bus as this seems like a bus
quirk..?
It's already part of sdw_bus
Right, but the point is that the properties were mostly derived from
DiSco, so am bit skeptical about it adding it there..
Oh, I am planning to contribute such quirks as MIPI DisCo properties for
the next revision of the document (along with a capability to disable a
link). This was not intended to remain Linux- or Intel-specific.
struct sdw_bus {
struct device *dev;
struct sdw_master_device *md;
unsigned int link_id;
int id;
struct list_head slaves;
DECLARE_BITMAP(assigned, SDW_MAX_DEVICES);
struct mutex bus_lock;
struct mutex msg_lock;
int (*compute_params)(struct sdw_bus *bus);
const struct sdw_master_ops *ops;
const struct sdw_master_port_ops *port_ops;
struct sdw_bus_params params;
struct sdw_master_prop prop;
The quirks could be set by a firmware property, and it seems logical to add
them at the same place where we already have properties defined in firmware,
no? That way all the standard, vendor-specific and quirks are read or added
in the same place.
Or they could be set by driver as well based on device id, compatible
and so on.. It maybe fw property as well, so limiting to property may not
be best idea IMO.
Not following, sorry. I didn't mean that only firmware can set them.
All of these sdw_master_prop can come from firmware or be hard-coded by
a driver for a specific implementation. The point is that they need to
be provided to the bus core so that it knows what to do.
If you look at DisCo today, we ignore the settings for the Slave
(unfortunately all wrong in BIOS) so the Slave properties are hard-coded
in drivers, but do use most of the firmware information for the Master,
so it's really firmware and/or driver that can set these properties.
the sdw_master_device isn't a good place for quirks IMHO, it's a very
shallow software-only layer without any existing ties to the hardware
definition.
This one I would agree.
