Message ID | 20181016150228.16994-7-nsaenzjulienne@suse.de |
---|---|
State | Rejected, archived |
Headers | show |
Series | staging: bcm2835-audio: Cleanups and upgrades | expand |
Context | Check | Description |
---|---|---|
robh/checkpatch | warning | "total: 0 errors, 1 warnings, 15 lines checked" |
Hi Nicolas, Am 16.10.2018 um 17:02 schrieb Nicolas Saenz Julienne: > Adds a device tree binding file for Raspberry pi's Headphones and HDMI > audio output devices. > > Based on raspberry's downstream kernel implementation: > https://github.com/raspberrypi/linux/blob/rpi-4.14.y/arch/arm/boot/dts/bcm2708-rpi.dtsi > > Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de> > --- > .../bindings/sound/brcm,bcm2835-audio.txt | 15 +++++++++++++++ > 1 file changed, 15 insertions(+) > create mode 100644 Documentation/devicetree/bindings/sound/brcm,bcm2835-audio.txt > > diff --git a/Documentation/devicetree/bindings/sound/brcm,bcm2835-audio.txt b/Documentation/devicetree/bindings/sound/brcm,bcm2835-audio.txt > new file mode 100644 > index 000000000000..ee6fa085aaa9 > --- /dev/null > +++ b/Documentation/devicetree/bindings/sound/brcm,bcm2835-audio.txt > @@ -0,0 +1,15 @@ > +Broadcom BCM283x audio device > + > +Required properties: > + > +- compatible: Should be "brcm,bcm2835-audio" > +- brcm,pwm-channels: number of PWM channels, they are behind RPi's Video Core > + IV, not actual Linux PWM devices. > + > +Example: > + > +audio: audio { > + compatible = "brcm,bcm2835-audio"; > + brcm,pwm-channels = <8>; > +}; > + i apologize but it seems to me that the TODO mentioned in the cover letter isn't update to date anymore. Phil Elwell posted an important bugfix for vchiq before [1], but only the driver part has been applied yet. After applying the DT changes i'm not sure if it still works. AFAIK the audio driver uses VCHIQ as a software interface and the binding doesn't describe the real hardware. Since the camera driver will be registered as a platform device [2], i prefer this way for the audio driver, too. I'm actually working on this here [3] (currently only compile tested). Stefan [1] - https://patchwork.ozlabs.org/cover/970434/ [2] - https://lore.kernel.org/patchwork/patch/904411/ [3] - https://github.com/anholt/linux/commits/bcm2835-audio
Hi Stefan, thanks for the review, On Tue, 2018-10-16 at 17:56 +0200, Stefan Wahren wrote: > Hi Nicolas, > > Am 16.10.2018 um 17:02 schrieb Nicolas Saenz Julienne: > > Adds a device tree binding file for Raspberry pi's Headphones and > > HDMI > > audio output devices. > > > > Based on raspberry's downstream kernel implementation: > > https://github.com/raspberrypi/linux/blob/rpi-4.14.y/arch/arm/boot/dts/bcm2708-rpi.dtsi > > > > Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de> > > --- > > .../bindings/sound/brcm,bcm2835-audio.txt | 15 > > +++++++++++++++ > > 1 file changed, 15 insertions(+) > > create mode 100644 > > Documentation/devicetree/bindings/sound/brcm,bcm2835-audio.txt > > > > diff --git a/Documentation/devicetree/bindings/sound/brcm,bcm2835- > > audio.txt b/Documentation/devicetree/bindings/sound/brcm,bcm2835- > > audio.txt > > new file mode 100644 > > index 000000000000..ee6fa085aaa9 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/sound/brcm,bcm2835- > > audio.txt > > @@ -0,0 +1,15 @@ > > +Broadcom BCM283x audio device > > + > > +Required properties: > > + > > +- compatible: Should be "brcm,bcm2835-audio" > > +- brcm,pwm-channels: number of PWM channels, they are behind RPi's > > Video Core > > + IV, not actual Linux PWM devices. > > + > > +Example: > > + > > +audio: audio { > > + compatible = "brcm,bcm2835-audio"; > > + brcm,pwm-channels = <8>; > > +}; > > + > > i apologize but it seems to me that the TODO mentioned in the cover > letter isn't update to date anymore. > > Phil Elwell posted an important bugfix for vchiq before [1], but only > the driver part has been applied yet. After applying the DT changes > i'm > not sure if it still works. > > AFAIK the audio driver uses VCHIQ as a software interface and the > binding doesn't describe the real hardware. > > Since the camera driver will be registered as a platform device [2], > i > prefer this way for the audio driver, too. > > I'm actually working on this here [3] (currently only compile > tested). > Fair enough. I wasn't feeling too good about the bindings myself. I only went with them because I saw something similar was happening between "raspberrypi,bcm2835-power" and "raspberrypi,bcm2835-firmware". I'll be glad to review & test your patches whenever they are ready. This makes commits 6 to 9 useless. Do you want me to send a v2? I can throw in an extra change Takashi suggested and update the TODO file. Regards, Nicolas
Hi Nicolas, > Nicolas Saenz Julienne <nsaenzjulienne@suse.de> hat am 16. Oktober 2018 um 18:48 geschrieben: > > > Hi Stefan, thanks for the review, > > On Tue, 2018-10-16 at 17:56 +0200, Stefan Wahren wrote: > > Hi Nicolas, > > > > Am 16.10.2018 um 17:02 schrieb Nicolas Saenz Julienne: > > > Adds a device tree binding file for Raspberry pi's Headphones and > > > HDMI > > > audio output devices. > > > > > > Based on raspberry's downstream kernel implementation: > > > > > https://github.com/raspberrypi/linux/blob/rpi-4.14.y/arch/arm/boot/dts/bcm2708-rpi.dtsi > > > > > > Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de> > > > --- > > > .../bindings/sound/brcm,bcm2835-audio.txt | 15 > > > +++++++++++++++ > > > 1 file changed, 15 insertions(+) > > > create mode 100644 > > > Documentation/devicetree/bindings/sound/brcm,bcm2835-audio.txt > > > > > > diff --git a/Documentation/devicetree/bindings/sound/brcm,bcm2835- > > > audio.txt b/Documentation/devicetree/bindings/sound/brcm,bcm2835- > > > audio.txt > > > new file mode 100644 > > > index 000000000000..ee6fa085aaa9 > > > --- /dev/null > > > +++ b/Documentation/devicetree/bindings/sound/brcm,bcm2835- > > > audio.txt > > > @@ -0,0 +1,15 @@ > > > +Broadcom BCM283x audio device > > > + > > > +Required properties: > > > + > > > +- compatible: Should be "brcm,bcm2835-audio" > > > +- brcm,pwm-channels: number of PWM channels, they are behind RPi's > > > Video Core > > > + IV, not actual Linux PWM devices. > > > + > > > +Example: > > > + > > > +audio: audio { > > > + compatible = "brcm,bcm2835-audio"; > > > + brcm,pwm-channels = <8>; > > > +}; > > > + > > > > i apologize but it seems to me that the TODO mentioned in the cover > > letter isn't update to date anymore. > > > > Phil Elwell posted an important bugfix for vchiq before [1], but only > > the driver part has been applied yet. After applying the DT changes > > i'm > > not sure if it still works. > > > > AFAIK the audio driver uses VCHIQ as a software interface and the > > binding doesn't describe the real hardware. > > > > Since the camera driver will be registered as a platform device [2], > > i > > prefer this way for the audio driver, too. > > > > I'm actually working on this here [3] (currently only compile > > tested). > > > > Fair enough. I wasn't feeling too good about the bindings myself. I > only went with them because I saw something similar was happening > between "raspberrypi,bcm2835-power" and "raspberrypi,bcm2835-firmware". > I'll be glad to review & test your patches whenever they are ready. i will inform you. > > This makes commits 6 to 9 useless. Do you want me to send a v2? I can > throw in an extra change Takashi suggested and update the TODO file. You can add my Acked-by: Stefan Wahren <stefan.wahren@i2se.com> to patches 1 to 5. Please drop 6 to 9 from the series and add the other suggestions. Btw please add devel@driverdev.osuosl.org to CC for V2 Thanks Stefan > > Regards, > Nicolas
diff --git a/Documentation/devicetree/bindings/sound/brcm,bcm2835-audio.txt b/Documentation/devicetree/bindings/sound/brcm,bcm2835-audio.txt new file mode 100644 index 000000000000..ee6fa085aaa9 --- /dev/null +++ b/Documentation/devicetree/bindings/sound/brcm,bcm2835-audio.txt @@ -0,0 +1,15 @@ +Broadcom BCM283x audio device + +Required properties: + +- compatible: Should be "brcm,bcm2835-audio" +- brcm,pwm-channels: number of PWM channels, they are behind RPi's Video Core + IV, not actual Linux PWM devices. + +Example: + +audio: audio { + compatible = "brcm,bcm2835-audio"; + brcm,pwm-channels = <8>; +}; +
Adds a device tree binding file for Raspberry pi's Headphones and HDMI audio output devices. Based on raspberry's downstream kernel implementation: https://github.com/raspberrypi/linux/blob/rpi-4.14.y/arch/arm/boot/dts/bcm2708-rpi.dtsi Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de> --- .../bindings/sound/brcm,bcm2835-audio.txt | 15 +++++++++++++++ 1 file changed, 15 insertions(+) create mode 100644 Documentation/devicetree/bindings/sound/brcm,bcm2835-audio.txt