Re: [PATCH v5] stm: class: Add MIPI OST protocol support

[Alexander Shishkin <alexander.shishkin@linux.intel.com>]

"Yingchao Deng (Consultant)" <quic_yingdeng@quicinc.com> writes:

> On 1/30/2026 5:48 PM, Alexander Shishkin wrote:
>> Yingchao Deng<yingchao.deng@oss.qualcomm.com> writes:
>>
>>> +	for (i = 1; i < ARRAY_SIZE(str_ost_entity_type); i++) {
>>> +		if (i == pn->entity_type)
>>> +			sz += sysfs_emit_at(page, sz, "[%s] ", str_ost_entity_type[i]);
>>> +		else
>>> +			sz += sysfs_emit_at(page, sz, "%s ", str_ost_entity_type[i]);
>>> +	}
>> Greg hates this. Documentation [0] says "preferably": "Attributes should
>> be ASCII text files, preferably with only one value per file.", but
>> somebody will get yelled at if this gets spotted, and since it's
>> probably going to be me, let's maybe not do this.
> Using two sysfs nodes, entity_available (listing supported options)
> and entity (the current value), is this acceptable?

My apologies, I didn't see that this was a configfs attribute (as it
should be).

Having said that, these ost types are almost enum stm_source_type. The
console is missing, but there is already a console source, so adding it
to the enum and the making the console source driver type that makes
sense. And once you have that, you'll get the entity type in your
ost_write() method via the @source parameter. Then, you'll only need a
tiny lookup table that maps enum stm_source_type to OST_ENTITY_* and use
that in the encoded packet.

This leaves the question of OST_ENTITY_TYPE_NONE and
OST_ENTITY_TYPE_DIAG. They are both unclear to me. OST_ENTITY_TYPE_NONE
basically means "no type, reject write", meaning that type 0 in an OST
packet is invalid, which seems weird and wasteful, is that what the
protocol spec says?

OST_ENTITY_TYPE_DIAG is also not explained; since we're already in the
tracing territory, "diag" may mean anything at all. From its value being
at the upper end of the 8-bit spectrum, I'm guessing it either something
very specialized or something very generic. Can it be a catch-all for
STM_USER (which is a catch-all)? Alternatively, you could propagate the
matched policy node's name to the pdrv->write() and in ost_write() check
if it's "diag" to select this type. strncmp() in pdrv->write() is not
ideal, though.

Either way, no extra attribute needed.

Because,

# mkdir diag ftrace console
# echo diag > diag/entity
# echo ftrace > ftrace/entity
# echo console > console/entity

is really a hat on a hat.

Would something like this work for you?

Thanks,
--
Alex