| Message ID | 20230315114806.3819515-1-cristian.ciocaltea@collabora.com |
|---|---|
| Headers | show |
| Series | Enable I2S support for RK3588/RK3588S SoCs | expand |
+Stephen On Wed, Mar 15, 2023 at 01:47:56PM +0200, Cristian Ciocaltea wrote: > Since commit df4fdd0db475 ("dt-bindings: firmware: arm,scmi: Restrict > protocol child node properties") the following dtbs_check warning is > shown: > > rk3588-rock-5b.dtb: scmi: protocol@14: Unevaluated properties are not allowed ('assigned-clock-rates', 'assigned-clocks' were unexpected) I think that's a somewhat questionable use of assigned-clock-rates. It should be located with the consumer rather than the provider IMO. The consumers of those 2 clocks are the CPU nodes. Rob
On Thu, Mar 16, 2023 at 03:34:17PM -0500, Rob Herring wrote: > +Stephen > > On Wed, Mar 15, 2023 at 01:47:56PM +0200, Cristian Ciocaltea wrote: > > Since commit df4fdd0db475 ("dt-bindings: firmware: arm,scmi: Restrict > > protocol child node properties") the following dtbs_check warning is > > shown: > > > > rk3588-rock-5b.dtb: scmi: protocol@14: Unevaluated properties are not allowed ('assigned-clock-rates', 'assigned-clocks' were unexpected) > > I think that's a somewhat questionable use of assigned-clock-rates. It > should be located with the consumer rather than the provider IMO. The > consumers of those 2 clocks are the CPU nodes. > Agreed. We definitely don't use those in the scmi clk provider driver. So NACK for the generic SCMI binding change.
On Fri, Mar 17, 2023 at 4:59 AM Cristian Ciocaltea <cristian.ciocaltea@collabora.com> wrote: > > On 3/17/23 00:26, Sudeep Holla wrote: > > On Thu, Mar 16, 2023 at 03:34:17PM -0500, Rob Herring wrote: > >> +Stephen > >> > >> On Wed, Mar 15, 2023 at 01:47:56PM +0200, Cristian Ciocaltea wrote: > >>> Since commit df4fdd0db475 ("dt-bindings: firmware: arm,scmi: Restrict > >>> protocol child node properties") the following dtbs_check warning is > >>> shown: > >>> > >>> rk3588-rock-5b.dtb: scmi: protocol@14: Unevaluated properties are not allowed ('assigned-clock-rates', 'assigned-clocks' were unexpected) > >> > >> I think that's a somewhat questionable use of assigned-clock-rates. It > >> should be located with the consumer rather than the provider IMO. The > >> consumers of those 2 clocks are the CPU nodes. > >> > > > > Agreed. We definitely don't use those in the scmi clk provider driver. > > So NACK for the generic SCMI binding change. > > According to [1], "configuration of common clocks, which affect multiple > consumer devices can be similarly specified in the clock provider node". True, but in this case it's really a single consumer because it's all CPU nodes which are managed together. > That would avoid duplicating assigned-clock-rates in the CPU nodes. Wouldn't one node be sufficient? Thinking more about this, why aren't you using OPP tables to define CPU frequencies. Assigned-clocks looks like a temporary hack because you haven't done proper OPP tables. Rob
On 3/17/23 16:27, Rob Herring wrote: > On Fri, Mar 17, 2023 at 4:59 AM Cristian Ciocaltea > <cristian.ciocaltea@collabora.com> wrote: >> >> On 3/17/23 00:26, Sudeep Holla wrote: >>> On Thu, Mar 16, 2023 at 03:34:17PM -0500, Rob Herring wrote: >>>> +Stephen >>>> >>>> On Wed, Mar 15, 2023 at 01:47:56PM +0200, Cristian Ciocaltea wrote: >>>>> Since commit df4fdd0db475 ("dt-bindings: firmware: arm,scmi: Restrict >>>>> protocol child node properties") the following dtbs_check warning is >>>>> shown: >>>>> >>>>> rk3588-rock-5b.dtb: scmi: protocol@14: Unevaluated properties are not allowed ('assigned-clock-rates', 'assigned-clocks' were unexpected) >>>> >>>> I think that's a somewhat questionable use of assigned-clock-rates. It >>>> should be located with the consumer rather than the provider IMO. The >>>> consumers of those 2 clocks are the CPU nodes. >>>> >>> >>> Agreed. We definitely don't use those in the scmi clk provider driver. >>> So NACK for the generic SCMI binding change. >> >> According to [1], "configuration of common clocks, which affect multiple >> consumer devices can be similarly specified in the clock provider node". > > True, but in this case it's really a single consumer because it's all > CPU nodes which are managed together. > >> That would avoid duplicating assigned-clock-rates in the CPU nodes. > > Wouldn't one node be sufficient? Yeah, that should be fine. > Thinking more about this, why aren't you using OPP tables to define > CPU frequencies. Assigned-clocks looks like a temporary hack because > you haven't done proper OPP tables. Right, this is currently not possible since it depends on some work in progress. Thanks, Cristian
On Wed, 15 Mar 2023 13:47:58 +0200, Cristian Ciocaltea wrote: > The ES8316 codec is currently used in conjunction with audio-graph-card > to provide an endpoint for binding with the other side of the audio > link. > > This is achieved via the 'port' property, which is not allowed: > > rk3399-rockpro64.dtb: codec@11: Unevaluated properties are not allowed ('port' was unexpected) > > Fix the issue by documenting the missing property. > > Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com> > --- > Documentation/devicetree/bindings/sound/everest,es8316.yaml | 4 ++++ > 1 file changed, 4 insertions(+) > Acked-by: Rob Herring <robh@kernel.org>
On Wed, 15 Mar 2023 13:48:00 +0200, Cristian Ciocaltea wrote: > Document the 'port' property to allow the Rockchip I2S TDM controller to > be used in conjunction with the audio-graph-card. > > The property will be used to provide an endpoint for binding to the > other side of the audio link. > > Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com> > --- > Documentation/devicetree/bindings/sound/rockchip,i2s-tdm.yaml | 4 ++++ > 1 file changed, 4 insertions(+) > Acked-by: Rob Herring <robh@kernel.org>
On Wed, 15 Mar 2023 13:48:02 +0200, Cristian Ciocaltea wrote: > Add new compatible string for the Rockchip I2S/PCM controller found on > RK3588 and RK3588S SoCs. > > Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com> > --- > Documentation/devicetree/bindings/sound/rockchip-i2s.yaml | 1 + > 1 file changed, 1 insertion(+) > Acked-by: Rob Herring <robh@kernel.org>
On Wed, 15 Mar 2023 13:47:55 +0200, Cristian Ciocaltea wrote: > There are five I2S/PCM/TDM controllers and two I2S/PCM controllers embedded > in the RK3588 and RK3588S SoCs. Furthermore, RK3588 provides four additional > I2S/PCM/TDM controllers. > > This patch series adds the required device tree nodes to support all the above. > > Additionally, it enables analog audio support for the Rock 5B SBC, which has > been used to test both audio playback and recording. > > [...] Applied to https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next Thanks! [03/11] ASoC: dt-bindings: everest,es8316: Document audio graph port commit: 2f43ef99ac623b6d9154c26d4f6785df18b4d8e4 [04/11] ASoC: dt-bindings: rockchip: Document audio graph port commit: bf4062b7420d01b4fafd7211fd2dc68b916591bd [05/11] ASoC: dt-bindings: rockchip: i2s-tdm: Document audio graph port commit: bfbae373c55e3b1c15a6ba656211dbbe7c390aa1 [06/11] ASoC: dt-bindings: rockchip: i2s-tdm: Document power-domains commit: 9971f3358338950d9d3345184fb2c0cfc6fc8552 [07/11] ASoC: dt-bindings: rockchip: Add compatible for RK3588 commit: b0fe6a91fa9d5599ba3cace2748906e086c5a56e [08/11] ASoC: rockchip: i2s: Add compatible for RK3588 commit: 0e6c37610934e9b91f6f5f2599de5e2f1ab59e72 All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted. You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed. If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced. Please add any relevant lists and maintainers to the CCs when replying to this mail. Thanks, Mark
There are five I2S/PCM/TDM controllers and two I2S/PCM controllers embedded in the RK3588 and RK3588S SoCs. Furthermore, RK3588 provides four additional I2S/PCM/TDM controllers. This patch series adds the required device tree nodes to support all the above. Additionally, it enables analog audio support for the Rock 5B SBC, which has been used to test both audio playback and recording. Note that some of the DT bindings fixes in this series are not particularly related to I2S, but are required for a proper dtbs_check validation. Cristian Ciocaltea (11): dt-bindings: firmware: arm,scmi: Document assigned-clocks and assigned-clock-rates dt-bindings: serial: snps-dw-apb-uart: Relax dma-names order constraint ASoC: dt-bindings: everest,es8316: Document audio graph port ASoC: dt-bindings: rockchip: Document audio graph port ASoC: dt-bindings: rockchip: i2s-tdm: Document audio graph port ASoC: dt-bindings: rockchip: i2s-tdm: Document power-domains ASoC: dt-bindings: rockchip: Add compatible for RK3588 ASoC: rockchip: i2s: Add compatible for RK3588 arm64: dts: rockchip: rk3588s: Add I2S nodes arm64: dts: rockchip: rk3588: Add I2S nodes arm64: dts: rockchip: rk3588-rock-5b: Add analog audio .../bindings/firmware/arm,scmi.yaml | 3 + .../bindings/serial/snps-dw-apb-uart.yaml | 10 +- .../bindings/sound/everest,es8316.yaml | 4 + .../bindings/sound/rockchip,i2s-tdm.yaml | 7 + .../bindings/sound/rockchip-i2s.yaml | 5 + .../boot/dts/rockchip/rk3588-rock-5b.dts | 62 ++++++++ arch/arm64/boot/dts/rockchip/rk3588.dtsi | 68 ++++++++ arch/arm64/boot/dts/rockchip/rk3588s.dtsi | 148 ++++++++++++++++++ sound/soc/rockchip/rockchip_i2s.c | 1 + 9 files changed, 305 insertions(+), 3 deletions(-)