> On Tue, Sep 03, 2024 at 02:44:33PM +0900, Kiseok Jo wrote:
> > Signed-off-by: Kiseok Jo <kiseok.jo@xxxxxxxxxxxxxx>
> > ---
> > sound/soc/codecs/Kconfig | 8 +
> > sound/soc/codecs/Makefile | 2 +
> > sound/soc/codecs/sma1307.c | 2191
> > ++++++++++++++++++++++++++++++++++++
> > sound/soc/codecs/sma1307.h | 454 ++++++++
> > 4 files changed, 2655 insertions(+)
> > create mode 100644 sound/soc/codecs/sma1307.c create mode 100644
> > sound/soc/codecs/sma1307.h
> >
>
> What are the differences against sm1303? Why it cannot be merged there?
> You have entire commit msg to explain this instead of just sending empty
> commit.
I will add a commit message in the next version of the patch.
> > + unsigned int tsdw_cnt;
> > +};
> > +
> > +static struct sma1307_pll_match sma1307_pll_matches[] = {
>
> const
Okay!
> > +
> > +/* DB scale conversion of speaker volume */ static const
> > +DECLARE_TLV_DB_SCALE(sma1307_spk_tlv, -6000, 50, 0);
> > +
> > +static int sma1307_regmap_write(struct sma1307_priv *sma1307,
> > + unsigned int reg, unsigned int val) {
> > + int ret = 0;
> > + int cnt = sma1307->retry_cnt;
> > +
> > + while (cnt--) {
>
> Sorry, but why? What is so broken in this hardware that it requires such
> retries? Maybe just youro board is broken, not this codec?
>
Generally, if there are no issues, retries shold not occur and it should execute only once.
Could this be a problem?
I included the retries because there might be issues with the connection.
> > +
> > +static void sma1307_remove(struct snd_soc_component *component) {
> > + struct sma1307_priv *sma1307 =
> > +snd_soc_component_get_drvdata(component);
> > +
> > + cancel_delayed_work_sync(&sma1307->check_fault_work);
>
> Why do you cancel work in two different places?
> > +
> > + sma1307 = devm_kzalloc(&client->dev,
> > + sizeof(struct sma1307_priv), GFP_KERNEL);
>
> sizeof(*)
>
Sorry. What does it mean?
> > +
> > + ret = devm_snd_soc_register_component(&client->dev,
> > + &sma1307_component, sma1307_dai,
> > + 1);
> > +
>
> Drop blank line
Okay, When using 'Lindent', line breaks like this.
I'll review and fix this.
Thanks!
> > +
> > +static const struct i2c_device_id sma1307_i2c_id[] = {
> > + { "sma1307", 0 },
> > + { }
> > +};
> > +
> > +MODULE_DEVICE_TABLE(i2c, sma1307_i2c_id);
> > +
> > +static const struct of_device_id sma1307_of_match[] = {
> > + {.compatible = "irondevice,sma1307a", },
> > + {.compatible = "irondevice,sma1307aq", }, //AEC-Q100 Qualificated
>
> Drop, useless. Also does not match your I2C ID table.
The SMA1307A and SMA1307AQ are different chips with different device settings.
Therefore, when registering in the devicetree, I intended to register the specific product and
Cofigure the device settings accordingly.
I set the I2C ID table to just 'sma1307' as shown.
So, should I change it to 'sma1307a' and 'sma1307aq instead?
> Anyway, bindings are before their users.
>
>
> > + { }
> > +/* SMA1307 Registers Bit Fields */
> > +/* Power On/Off */
> > +#define SMA1307_POWER_MASK (1<<0)
> > +#define SMA1307_POWER_OFF (0<<0)
> > +#define SMA1307_POWER_ON (1<<0)
>
> Use BIT() everywhere.
>
Okay, thanks!
> Best regards,
> Krzysztof
