Expand SoundWire MBQ register map support #5268
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Compliment the existing macro to construct an SDCA control address with macros to extract the constituent parts, and validation of such an address. Also update the masks for the original macro to use GENMASK to make mental comparisons with the included comment on the address format easier. Signed-off-by: Charles Keepax <[email protected]>
Update the list of entity_0 controls to better match version v1.0 of the SDCA specification. Remove both INTSTAT_CLEAR and INT_ENABLE as these are no longer used, and add some missing controls and bits into the enum. Also rename the SDCA_CONTROL prefix to SDCA_CTL because this better matches the macros in the sdw_registers.h header, and the names can get quite long so saving a few characters is definitely a plus. Signed-off-by: Charles Keepax <[email protected]>
charleskeepax
requested review from
lgirdwood,
plbossart,
ranj063,
kv2019i,
dbaluta,
bardliao and
ujfalusi
as code owners
bardliao
reviewed
Dec 10, 2024
| return read1; | ||
| read = sdw_read_no_pm(slave, reg); | ||
| if (read < 0) | ||
| return read; |
There was a problem hiding this comment.
Don't we need *val = read; after this?
There was a problem hiding this comment.
Indeed yes, thank you very much for spotting that. I will get it fixed up and do a respin.
| return read; | ||
| } | ||
|
|
||
| *val = read; |
There was a problem hiding this comment.
Oh, you add it in this commit. IMHO, this belongs to the regmap: sdw-mbq: Add support for further MBQ register sizes commit.
SoundWire MBQ register maps typically contain a variety of register sizes, which doesn't map ideally to the regmap abstraction which expects register maps to have a consistent size. Currently the MBQ register map only allows 16-bit registers to be defined, however this leads to complex CODEC driver implementations with an 8-bit register map and a 16-bit MBQ, every control will then have a custom get and put handler that allows them to access different register maps. Further more 32-bit MBQ quantities are not currently supported. Add support for additional MBQ sizes and to avoid the complexity of multiple register maps treat the val_size as a maximum size for the register map. Within the regmap use an ancillary callback to determine how many bytes to actually read/write to the hardware for a specific register. In the case that no callback is defined the behaviour defaults back to the existing behaviour of a fixed size register map. Signed-off-by: Charles Keepax <[email protected]>
The SDCA specification allows for controls to be deferred. In the case of a deferred control the device will return COMMAND_IGNORED to the 8-bit operation that would cause the value to commit. Which is the final 8-bits on a write, or the first 8-bits on a read. In the case of receiving a defer, the regmap will poll the SDCA function busy bit, after which the transaction will be retried, returning an error if the function busy does not clear within a chip specific timeout. Since this is common SDCA functionality which is the 99% use-case for MBQs it makes sense to incorporate this functionality into the register map. If no MBQ configuration is specified, the behaviour will default to the existing behaviour. Signed-off-by: Charles Keepax <[email protected]>
Now the MBQ regmap implementation handles multiple sizes, this driver can combine its two register maps into one. So remove mbq_regmap and combine all the registers into regmap. Also as rt722_sdca_adc_mux_get/put() only exist to access mbq_regmap, rather than doing any processing, these can now be dropped and the normal DAPM helpers used. Signed-off-by: Charles Keepax <[email protected]>
charleskeepax
force-pushed
the
sof/sdca
branch
from
10ce156 to
6eacbc5
Compare
lgirdwood
reviewed
Dec 10, 2024
| *val = (read1 << 8) | read0; | ||
| *val = read; | ||
|
|
||
| while (--mbq_size > 0) { |
There was a problem hiding this comment.
Will this ever be large and block userspace ?
There was a problem hiding this comment.
Should be fine in regmap_sdw_mbq_size we do:
if (!size || size > ctx->val_size)
return -EINVAL;
And we don't allow val_size to get too big in regmap_sdw_mbq_config_check:
if (config->val_bits > (sizeof(unsigned int) * BITS_PER_BYTE))
return -ENOTSUPP;
So the largest mbq_size can ever be is sizeof(unsigned int).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The current MBQ register map only supports 16-bit types, add support for more sizes and then update the rt722 driver to use the new support. Obviously I don't have hardware to test the rt722 change, but thought it was good to include a change to demonstrate the new features in use. If you guys had an rt722 for a quick test that would be awesome.