Re: [PATCH v8 3/3] ASoC: qcom: sc7180: Add machine driver for sound card registration

On Thu, Sep 10, 2020 at 1:24 PM Cheng-Yi Chiang cychiang@chromium.org wrote:

+struct sc7180_snd_data {

•   u32 pri_mi2s_clk_count;
  

•   struct snd_soc_jack hs_jack;
  

•   struct device_node *hs_jack_of_node;
  

•   struct snd_soc_jack hdmi_jack;
  

•   struct device_node *hdmi_jack_of_node;
  

+};

hs_jack_of_node and hdmi_jack_of_node are not using. Remove them.

+static int sc7180_init(struct snd_soc_pcm_runtime *rtd) +{

•   struct snd_soc_dai *cpu_dai = asoc_rtd_to_cpu(rtd, 0);
  

•   switch (cpu_dai->id) {
  

•   case MI2S_PRIMARY:
  

•           return sc7180_headset_init(rtd);
  

•   case MI2S_SECONDARY:
  

•           return 0;
  

•   case HDMI:
  

I guess this enumeration has not merged yet? It doesn't sound like a specific-enough naming.

+static int sc7180_snd_platform_probe(struct platform_device *pdev) +{

•   struct snd_soc_card *card = &sc7180_card;
  

•   struct sc7180_snd_data *data;
  

•   struct device *dev = &pdev->dev;
  

•   int ret;
  

•   /* Allocate the private data */
  

•   data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
  

•   if (!data)
  

•           return -ENOMEM;
  

•   card->dev = dev;
  

•   ret = qcom_snd_parse_of(card);
  

•   if (ret) {
  

•           dev_err(dev, "Error parsing OF data\n");
  

•           return ret;
  

•   }
  

•   snd_soc_card_set_drvdata(card, data);
  

To be neat, move this line immediately before or after "card->dev = dev;".