linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Sebastian Reichel <sebastian.reichel@collabora.com>
To: Samuel Holland <samuel@sholland.org>
Cc: Rob Herring <robh+dt@kernel.org>,
	devicetree@vger.kernel.org, linux-pm@vger.kernel.org,
	linux-kernel@vger.kernel.org, Ondrej Jirman <megous@megous.com>
Subject: Re: [PATCH 3/3] power: supply: Add a driver for Injoinic power bank ICs
Date: Fri, 11 Feb 2022 22:22:18 +0100	[thread overview]
Message-ID: <20220211212218.j5r75tyvhirxis4z@mercury.elektranox.org> (raw)
In-Reply-To: <20220129222424.45707-3-samuel@sholland.org>

[-- Attachment #1: Type: text/plain, Size: 2772 bytes --]

Hi,

On Sat, Jan 29, 2022 at 04:24:24PM -0600, Samuel Holland wrote:
> This driver supports several chip variants which all share the same I2C
> register interface. Since the chip will turn off and become inaccessible
> under conditions outside of software control (e.g. upon button press or
> input voltage removal), some special handling is needed to delay the
> initialization of the IC until it is accessible.
> 
> Signed-off-by: Samuel Holland <samuel@sholland.org>
> ---

Thanks, driver looks mostly good to me. Just two things:

> [...]
> +static const struct power_supply_desc ip5xxx_charger_desc = {
> +	.name			= "ip5xxx-charger",
> +	.type			= POWER_SUPPLY_TYPE_BATTERY,

POWER_SUPPLY_TYPE_BATTERY is the wrong type for a charger.
Considering the chips want 5V on the input side as far as
I could see, a sensible type is POWER_SUPPLY_TYPE_USB.

> +	.properties		= ip5xxx_charger_properties,
> +	.num_properties		= ARRAY_SIZE(ip5xxx_charger_properties),
> +	.get_property		= ip5xxx_charger_get_property,
> +	.set_property		= ip5xxx_charger_set_property,
> +	.property_is_writeable	= ip5xxx_charger_property_is_writeable,
> +};
> +
> +static const struct regmap_config ip5xxx_regmap_config = {
> +	.reg_bits		= 8,
> +	.val_bits		= 8,
> +	.max_register		= IP5XXX_BATOCV_DAT1,
> +};
> +
> +static int ip5xxx_power_probe(struct i2c_client *client)
> +{
> +	struct power_supply_config psy_cfg = {};
> +	struct device *dev = &client->dev;
> +	struct power_supply *psy;
> +	struct ip5xxx *ip5xxx;
> +
> +	ip5xxx = devm_kzalloc(dev, sizeof(*ip5xxx), GFP_KERNEL);

if (!ip5xxx)
    return -ENOMEM;

-- Sebastian

> +	ip5xxx->regmap = devm_regmap_init_i2c(client, &ip5xxx_regmap_config);
> +	if (IS_ERR(ip5xxx->regmap))
> +		return PTR_ERR(ip5xxx->regmap);
> +
> +	psy_cfg.of_node = dev->of_node;
> +	psy_cfg.drv_data = ip5xxx;
> +
> +	psy = devm_power_supply_register(dev, &ip5xxx_charger_desc, &psy_cfg);
> +	if (IS_ERR(psy))
> +		return PTR_ERR(psy);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id ip5xxx_power_of_match[] = {
> +	{ .compatible = "injoinic,ip5108" },
> +	{ .compatible = "injoinic,ip5109" },
> +	{ .compatible = "injoinic,ip5207" },
> +	{ .compatible = "injoinic,ip5209" },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, ip5xxx_power_of_match);
> +
> +static struct i2c_driver ip5xxx_power_driver = {
> +	.probe_new	= ip5xxx_power_probe,
> +	.driver		= {
> +		.name		= "ip5xxx-power",
> +		.of_match_table	= ip5xxx_power_of_match,
> +	}
> +};
> +module_i2c_driver(ip5xxx_power_driver);
> +
> +MODULE_AUTHOR("Samuel Holland <samuel@sholland.org>");
> +MODULE_DESCRIPTION("Injoinic IP5xxx power bank IC driver");
> +MODULE_LICENSE("GPL");
> -- 
> 2.33.1
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

  reply	other threads:[~2022-02-11 21:22 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-01-29 22:24 [PATCH 1/3] dt-bindings: vendor-prefixes: Add Injoinic Samuel Holland
2022-01-29 22:24 ` [PATCH 2/3] dt-bindings: trivial-devices: Add Injoinic power bank ICs Samuel Holland
2022-02-11 13:07   ` Rob Herring
2022-01-29 22:24 ` [PATCH 3/3] power: supply: Add a driver for " Samuel Holland
2022-02-11 21:22   ` Sebastian Reichel [this message]
2022-02-14  4:20     ` Samuel Holland
2022-02-11 13:06 ` [PATCH 1/3] dt-bindings: vendor-prefixes: Add Injoinic Rob Herring

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220211212218.j5r75tyvhirxis4z@mercury.elektranox.org \
    --to=sebastian.reichel@collabora.com \
    --cc=devicetree@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=megous@megous.com \
    --cc=robh+dt@kernel.org \
    --cc=samuel@sholland.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).