[v2,0/2] add mcp4728 I2C DAC driver​

Message

Andrea Collamati July 16, 2023, 9:26 p.m. UTC

Dear Krzysztof

I release a V2 version. I hope I have understood all your requests.
See below.

> 2. Please use scripts/get_maintainers.pl to get a list of necessary
> people and lists to CC (and consider --no-git-fallback argument). It
> might happen, that command when run on an older kernel, gives you
> outdated entries. Therefore please be sure you base your patches on
> recent Linux kernel.
>
I was using branch master git://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git but

I saw it was too old. I switched to branch testing.
> 6. mcp or MCP? What is this? Proper title is missing... also no
> description.
>
I don't know whether using small o capital letters. Usually manufacturers use capital letter to identify the component
but kernel drivers use lowercase in code.
>
> Shouldn't this binding be just merged with existing mcp4725? Are you
> sure it's not similar device, IOW, are you sure you do not have vref supply?
>
Unfortunately they are not that similar. MCP4728 has two vref choices:

- Use Vdd (power supply) as Vref

- User Internal 2.048 V Vref source.


Andrea Collamati (2):
  dt-bindings: iio: dac: add mcp4728.yaml
  iio: add mcp4728 I2C DAC driver

 .../bindings/iio/dac/microchip,mcp4728.yaml   | 42 +++++++++
 drivers/iio/dac/mcp4728.c                     | 93 +++++++++----------
 2 files changed, 86 insertions(+), 49 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/iio/dac/microchip,mcp4728.yaml

Comments

Krzysztof Kozlowski July 17, 2023, 7:17 a.m. UTC | #1

On 16/07/2023 23:26, Andrea Collamati wrote:
> Dear Krzysztof
> 
> I release a V2 version. I hope I have understood all your requests.
> See below.
> 
>> 2. Please use scripts/get_maintainers.pl to get a list of necessary
>> people and lists to CC (and consider --no-git-fallback argument). It
>> might happen, that command when run on an older kernel, gives you
>> outdated entries. Therefore please be sure you base your patches on
>> recent Linux kernel.
>>
> I was using branch master git://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git but

Should be fine. I still doubt that you used it, though. If you got
Conor's email, then for sure you should have got my email.

> 
> I saw it was too old. I switched to branch testing.
>> 6. mcp or MCP? What is this? Proper title is missing... also no
>> description.
>>
> I don't know whether using small o capital letters. Usually manufacturers use capital letter to identify the component
> but kernel drivers use lowercase in code.

Use what is in datasheet.

>>
>> Shouldn't this binding be just merged with existing mcp4725? Are you
>> sure it's not similar device, IOW, are you sure you do not have vref supply?
>>
> Unfortunately they are not that similar. MCP4728 has two vref choices:
> 
> - Use Vdd (power supply) as Vref
> 
> - User Internal 2.048 V Vref source.

Exactly the same as existing mcp4726, so what is the problem?


Best regards,
Krzysztof

Andrea Collamati July 17, 2023, 12:23 p.m. UTC | #2

On 7/17/23 09:17, Krzysztof Kozlowski wrote:
>>> Shouldn't this binding be just merged with existing mcp4725? Are you
>>> sure it's not similar device, IOW, are you sure you do not have vref supply?
>>>
>> Unfortunately they are not that similar. MCP4728 has two vref choices:
>>
>> - Use Vdd (power supply) as Vref
>>
>> - User Internal 2.048 V Vref source.
> Exactly the same as existing mcp4726, so what is the problem?

MCP4726 has 2 separate pins, VDD and VREF that can be both used as external Vref depending on configuration.

MCP4728 has only one external vref pin, VDD pin. The other vref source is internal and fixed to 2.048V.

Furthermore MCP4728  has four independent channels. For each channels it's possible to choose whether

using VDD (External VRef) or 2.048V (Internal Vref).

Best regards

            Andrea

Krzysztof Kozlowski July 17, 2023, 3:03 p.m. UTC | #3

On 17/07/2023 14:23, Andrea Collamati wrote:
> On 7/17/23 09:17, Krzysztof Kozlowski wrote:
>>>> Shouldn't this binding be just merged with existing mcp4725? Are you
>>>> sure it's not similar device, IOW, are you sure you do not have vref supply?
>>>>
>>> Unfortunately they are not that similar. MCP4728 has two vref choices:
>>>
>>> - Use Vdd (power supply) as Vref
>>>
>>> - User Internal 2.048 V Vref source.
>> Exactly the same as existing mcp4726, so what is the problem?
> 
> MCP4726 has 2 separate pins, VDD and VREF that can be both used as external Vref depending on configuration.
> 
> MCP4728 has only one external vref pin, VDD pin. The other vref source is internal and fixed to 2.048V.
> 
> Furthermore MCP4728  has four independent channels. For each channels it's possible to choose whether
> 
> using VDD (External VRef) or 2.048V (Internal Vref).

Indeed, so new binding makes sense.

Best regards,
Krzysztof