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.

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

drivers/tca64xx: Add support for PCAL6416 #312

Merged
merged 1 commit into from
Nov 7, 2024
Merged

drivers/tca64xx: Add support for PCAL6416 #312

merged 1 commit into from
Nov 7, 2024

Conversation

joukkone
Copy link

@joukkone joukkone commented Nov 6, 2024

New part support added to driver,
pullup/pulldown configuration support

Summary

Impact

Testing

if ((direction == IOEXPANDER_DIRECTION_IN_PULLDOWN) ||
(direction == IOEXPANDER_DIRECTION_IN_PULLUP))
{
regaddr = tca64_pdenable_reg(priv, pin);
Copy link

@jlaitine jlaitine Nov 6, 2024

Choose a reason for hiding this comment

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

There is no error handling now if PU/PD setting is attempted for other parts than the one which has the register. You should propably add "define TCA6424_INVALID_REG 0xff" or such in header, and make tca64_pdenable_reg return that if part->tp_pu_select is 0.

Then you can check the regaddr here (!= TCA6424_INVALID_REG) and bail out if the part doesn't support PU/PD....

Copy link
Author

Choose a reason for hiding this comment

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

It should be fixed now.

@joukkone joukkone force-pushed the pcal6416 branch 2 times, most recently from 7b1f88b to 1ec4ea4 Compare November 6, 2024 10:58
FAR const struct tca64_part_s *part = tca64_getpart(priv);
uint8_t reg = part->tp_puenable;

if (reg == TC64XX_INVALID_REG)
Copy link

@jlaitine jlaitine Nov 6, 2024

Choose a reason for hiding this comment

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

If you do other way around;

if (reg === 0) return TC64XX_INVALID_REG;

This makes more sense. Now this returns 0, which is a valid register address.... even though never pdenable register.

or, if (reg == TC64XX_INVALID_REG) return reg;

Copy link
Author

Choose a reason for hiding this comment

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

done

FAR const struct tca64_part_s *part = tca64_getpart(priv);
uint8_t reg = part->tp_pu_select;

if (reg == TC64XX_INVALID_REG)
Copy link

Choose a reason for hiding this comment

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

the same comment as above

(direction == IOEXPANDER_DIRECTION_IN_PULLUP))
{
regaddr = tca64_pdenable_reg(priv, pin);
if (regaddr == 0)
Copy link

Choose a reason for hiding this comment

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

And here if (regaddr == TC64XX_INVALID_REG) ....

@jlaitine
Copy link

jlaitine commented Nov 6, 2024

What you did works; it is just a bit confusing because it returns regaddr, which is valid (0), and you just know that 0 cannot be right one. Rather return the invalid address all the way to the place where it is queried.

New part support added to driver,
pullup/pulldown configuration support

Signed-off-by: Jouni Ukkonen <[email protected]>
@joukkone joukkone merged commit 27ee0b4 into master Nov 7, 2024
11 checks passed
@joukkone joukkone deleted the pcal6416 branch November 7, 2024 06:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants