Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Driver for Allo Katana DAC #2519

Merged
merged 4 commits into from
Apr 25, 2018
Merged

Driver for Allo Katana DAC #2519

merged 4 commits into from
Apr 25, 2018

Conversation

allocom
Copy link

@allocom allocom commented Apr 19, 2018

Allo Katana DAC uses Sabre ESS codec.

@clivem
Copy link

clivem commented Apr 19, 2018 •
edited
Loading

Have ESS changed their stance? AFAIK, there is still the issue for any of their programmable Sabre chips, (ie. I2C software controlled), that you sign a NDA to get the register info/address/values.
Unless they have changed their stance, they have forbidden any Open Source driver where any register address or programming value, that is obtained from a NDA'd datasheet, being exposed in an Open Source driver! Maybe you know this, maybe you don't. Just a friendly warning before you potentially drag the RPi guys into a legal nightmare with ESS, for "publishing" such a driver......

@HiassofT
Copy link
Contributor

The soundcard driver (allo-katana-dac.c) doesn't seem to do much, you can drop it and use the simple card or audio graph card driver instead.

Here's a (completely untested) dt overlay, please give that a try: https://gist.github.com/HiassofT/5f7aec2407f854e184aecdc7d502779e

@pelwell
Copy link
Contributor

pelwell commented Apr 19, 2018

[ Comment 1 has been largely preempted before submission, but still stands. ]

  1. I believe you may be violating your NDA by publishing this driver - do you really want to do this?
  2. The codec is a standard component and its driver should be upstreamed.

@AlloKatana
Copy link

Hi,

I am one of the designers of the Katana Dac (using ess9038q2m). We are very well aware of the NDA from ess sabre (that we signed). So we used the only solution possible. Katana Dac has an MCU on board with the actual protected information (nda) of Ess Sabre. The driver that we are submitting is the driver of the MCU (and the translation is done securely in the MCU). Let me know if you have any other questions.

@pelwell
Copy link
Contributor

pelwell commented Apr 19, 2018

So is the codec a standard component or have you modified the MCU firmware for your application?

Is the API exposed by the MCU in the public domain?

@HiassofT
Copy link
Contributor

If the codec driver (sabre-ess-i2c) isn't talking to the ESS chip but your MCU then you should name it differently - allo-ess-bridge-i2c or sth like that

@allocom
Copy link
Author

allocom commented Apr 20, 2018

Hi Pelwell, Codec is not a standard component. MCU firmware is custom written & will not be made public. Register addresses & its definition are not directly mapped to the ESS chip registers.

@pelwell
Copy link
Contributor

pelwell commented Apr 20, 2018

In that case you should:

  1. delete the sound card driver and use the overlay suggested by HiassofT, and
  2. move the codec driver into sound/soc/bcm, giving it a product-specific name - allo_katana_codec.c would be fine.

@allocom
Copy link
Author

allocom commented Apr 20, 2018

I am checking the overlay suggested by HiassofT (Thanks HiassofT) & will move the codec driver.

@allocom
Copy link
Author

allocom commented Apr 24, 2018

Hello Pelwell, I modified to have only one file (allo_katana_codec.c) under bcm. Pls check if it is fine.

pelwell
pelwell reviewed Apr 24, 2018
View reviewed changes
Copy link
Contributor

@pelwell pelwell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apart from my README comments, that looks good.

arch/arm/boot/dts/overlays/README Outdated
@@ -314,6 +314,12 @@ Load: dtoverlay=allo-digione
Params: <None>


Name: allo-katana
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be allo-katana-dac-audio

arch/arm/boot/dts/overlays/README Outdated
@@ -314,6 +314,12 @@ Load: dtoverlay=allo-digione
Params: <None>


Name: allo-katana
Info: Configures the Allo Katana audio card
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Allo Katana DAC audio card for consistency? You can choose.

@allocom
Copy link
Author

allocom commented Apr 25, 2018

Modified README file.

@pelwell pelwell merged commit 2b90676 into raspberrypi:rpi-4.14.y Apr 25, 2018
@AlloKatana
Copy link

Thx you Pelwell for merging, HiassofT for the suggestions and CliveM for the comment (valid).

@pelwell
Copy link
Contributor

pelwell commented Apr 25, 2018

I used the wrong merge option - I intended to squash them - but I'll fix it up in the other kernel versions.

@allocom
Copy link
Author

allocom commented Apr 25, 2018

Thanks all

popcornmix added a commit to raspberrypi/firmware that referenced this pull request Apr 26, 2018
@popcornmix
kernel: Bump to 4.14.37
2b0849b 
kernel: BCM270X_DT: Add sdio_overclock parameter

kernel: gpiolib: Don't prevent IRQ usage of output GPIOs
See: raspberrypi/linux#2527

kernel: Driver for Allo Katana DAC
See: raspberrypi/linux#2519

kernel: Prevent voltage low warnings from filling log
See: raspberrypi/linux#2520

kernel: overlays: Add 'combine' option to i2c overlays
See: #828

firmware: Extra reg writes to ensure the LCD display starts up at 800 wide

firmware: platform: Lower temperature thresholds for extra pips to 50/60

firmware: video_encode: Add support for YVU420Planar and YVU420SemiPlanar
firmware: video_decode: Support YV12, NV12, and NV21 output
firmware: video_decode: Support reporting colour space
firmware: IL rawcam: Fix copy/paste error on timing setup
firmware: MMAL: Populate buffer header TYPE_SPECIFIC fields
popcornmix added a commit to Hexxeh/rpi-firmware that referenced this pull request Apr 26, 2018
@popcornmix
kernel: Bump to 4.14.37
8f3b0aa 
kernel: BCM270X_DT: Add sdio_overclock parameter

kernel: gpiolib: Don't prevent IRQ usage of output GPIOs
See: raspberrypi/linux#2527

kernel: Driver for Allo Katana DAC
See: raspberrypi/linux#2519

kernel: Prevent voltage low warnings from filling log
See: raspberrypi/linux#2520

kernel: overlays: Add 'combine' option to i2c overlays
See: raspberrypi/firmware#828

firmware: Extra reg writes to ensure the LCD display starts up at 800 wide

firmware: platform: Lower temperature thresholds for extra pips to 50/60

firmware: video_encode: Add support for YVU420Planar and YVU420SemiPlanar
firmware: video_decode: Support YV12, NV12, and NV21 output
firmware: video_decode: Support reporting colour space
firmware: IL rawcam: Fix copy/paste error on timing setup
firmware: MMAL: Populate buffer header TYPE_SPECIFIC fields
popcornmix added a commit to raspberrypi/firmware that referenced this pull request Apr 26, 2018
@popcornmix
kernel: Bump to 4.14.37
983b091 
kernel: BCM270X_DT: Add sdio_overclock parameter

kernel: gpiolib: Don't prevent IRQ usage of output GPIOs
See: raspberrypi/linux#2527

kernel: Driver for Allo Katana DAC
See: raspberrypi/linux#2519

kernel: Prevent voltage low warnings from filling log
See: raspberrypi/linux#2520

kernel: overlays: Add 'combine' option to i2c overlays
See: #828

firmware: Extra reg writes to ensure the LCD display starts up at 800 wide

firmware: platform: Lower temperature thresholds for extra pips to 50/60

firmware: video_encode: Add support for YVU420Planar and YVU420SemiPlanar
firmware: video_decode: Support YV12, NV12, and NV21 output
firmware: video_decode: Support reporting colour space
firmware: IL rawcam: Fix copy/paste error on timing setup
firmware: MMAL: Populate buffer header TYPE_SPECIFIC fields
popcornmix added a commit to Hexxeh/rpi-firmware that referenced this pull request Apr 26, 2018
@popcornmix
kernel: Bump to 4.14.37
461ee53 
kernel: BCM270X_DT: Add sdio_overclock parameter

kernel: gpiolib: Don't prevent IRQ usage of output GPIOs
See: raspberrypi/linux#2527

kernel: Driver for Allo Katana DAC
See: raspberrypi/linux#2519

kernel: Prevent voltage low warnings from filling log
See: raspberrypi/linux#2520

kernel: overlays: Add 'combine' option to i2c overlays
See: raspberrypi/firmware#828

firmware: Extra reg writes to ensure the LCD display starts up at 800 wide

firmware: platform: Lower temperature thresholds for extra pips to 50/60

firmware: video_encode: Add support for YVU420Planar and YVU420SemiPlanar
firmware: video_decode: Support YV12, NV12, and NV21 output
firmware: video_decode: Support reporting colour space
firmware: IL rawcam: Fix copy/paste error on timing setup
firmware: MMAL: Populate buffer header TYPE_SPECIFIC fields
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pelwell pelwell pelwell left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@allocom @clivem @HiassofT @pelwell @AlloKatana