Re: [PATCH v3 08/14] ASoC: SOF: Add DSP HW abstraction operations

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 1/9/19 2:51 PM, Mark Brown wrote:
On Tue, Dec 11, 2018 at 03:23:12PM -0600, Pierre-Louis Bossart wrote:

+int snd_sof_pci_update_bits_unlocked(struct snd_sof_dev *sdev, u32 offset,
+				     u32 mask, u32 value)
+{
+	bool change;
+	unsigned int old, new;
+	u32 ret = ~0; /* explicit init to remove uninitialized use warnings */
This looks a lot like you want to write regmap_pci_config...

I think you made that note for the v2 a long time ago, not sure if I replied at the time.

We only use this for 4 cases (power/clock gating on/off, traffic class, etc) and only during the hardware initialization. This is similar to the legacy and Skylake driver, I don't see a significant benefit with a regmap?

intel/hda-ctrl.c:       snd_sof_pci_update_bits(sdev, PCI_CGCTL, PCI_CGCTL_MISCBDCGE_MASK, val); intel/hda-ctrl.c:       snd_sof_pci_update_bits(sdev, PCI_CGCTL, PCI_CGCTL_ADSPDCGE, val); intel/hda-ctrl.c:       snd_sof_pci_update_bits(sdev, PCI_PGCTL, PCI_PGCTL_ADSPPGD, val);
intel/hda-dsp.c:        snd_sof_pci_update_bits(sdev, PCI_PGCTL,
intel/hda-dsp.c:        snd_sof_pci_update_bits(sdev, PCI_TCSEL, 0x07, 0);


+/* control */
+static inline int snd_sof_dsp_run(struct snd_sof_dev *sdev)
+{
+	if (sdev->ops->run)
+		return sdev->ops->run(sdev);
+
+	return 0;
+}
Do we really want to return 0 for all these ops if they're not
implemented?  For some that seems sensible but there's others where it
seems like the caller might want to know they got ignored and an error
code like -ENOTSUPP might be better.
Good point indeed. There are a set of ops that are really mandatory, we should rework this instead. Thanks for the comment.
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel




[Index of Archives]     [ALSA User]     [Linux Audio Users]     [Pulse Audio]     [Kernel Archive]     [Asterisk PBX]     [Photo Sharing]     [Linux Sound]     [Video 4 Linux]     [Gimp]     [Yosemite News]

  Powered by Linux