On Tue, Mar 31, 2020 at 03:28:17PM +0300, Matti Vaittinen wrote:
> The ROHM BD99954 is a Battery Management LSI for 1-4 cell Lithium-Ion
> secondary battery intended to be used in space-constraint equipment such
> as Low profile Notebook PC, Tablets and other applications. BD99954
> provides a Dual-source Battery Charger, two port BC1.2 detection and a
> Battery Monitor.
>
> Support ROHM BD99954 Charger IC.
...
> +static irqreturn_t bd9995x_irq_handler_thread(int irq, void *private)
> +{
> + struct bd9995x_device *bd = private;
> + int ret, status, mask, i;
> + unsigned long tmp;
> + struct bd9995x_state state;
> +
> + /*
> + * The bd9995x does not seem to generate big amount of interrupts.
> + * The logic regarding which interrupts can cause relevant
> + * status changes seem to be pretty complex.
> + *
> + * So lets implement really simple and hopefully bullet-proof handler:
> + * It does not really matter which IRQ we handle, we just go and
> + * re-read all interesting statuses + give the framework a nudge.
> + *
> + * Other option would be building a _complex_ and error prone logic
> + * trying to decide what could have been changed (resulting this IRQ
> + * we are now handling). During the normal operation the BD99954 does
> + * not seem to be generating much of interrupts so benefit from such
> + * logic would probably be minimal.
> + */
> +
> + ret = regmap_read(bd->rmap, INT0_STATUS, &status);
> + if (ret) {
> + dev_err(bd->dev, "Failed to read IRQ status\n");
> + return IRQ_NONE;
> + }
> +
> + ret = regmap_field_read(bd->rmap_fields[F_INT0_SET], &mask);
> + if (ret) {
> + dev_err(bd->dev, "Failed to read IRQ mask\n");
> + return IRQ_NONE;
> + }
> +
> + /* Handle only IRQs that are not masked */
> + status &= mask;
> + tmp = status;
> +
> + /* Lowest bit does not represent any sub-registers */
> + tmp >>= 1;
> +
> + /*
> + * Mask and ack IRQs we will handle (+ the idiot bit)
> + */
> + ret = regmap_field_write(bd->rmap_fields[F_INT0_SET], 0);
> + if (ret) {
> + dev_err(bd->dev, "Failed to mask F_INT0\n");
> + return IRQ_NONE;
> + }
> +
> + ret = regmap_write(bd->rmap, INT0_STATUS, status);
> + if (ret) {
> + dev_err(bd->dev, "Failed to ack F_INT0\n");
> + goto err_umask;
> + }
> +
> + for_each_set_bit(i, &tmp, 7) {
> + int sub_status, sub_mask;
> + int sub_status_reg[] = {
> + INT1_STATUS, INT2_STATUS, INT3_STATUS, INT4_STATUS,
> + INT5_STATUS, INT6_STATUS, INT7_STATUS,
> + };
> + struct regmap_field *sub_mask_f[] = {
> + bd->rmap_fields[F_INT1_SET],
> + bd->rmap_fields[F_INT2_SET],
> + bd->rmap_fields[F_INT3_SET],
> + bd->rmap_fields[F_INT4_SET],
> + bd->rmap_fields[F_INT5_SET],
> + bd->rmap_fields[F_INT6_SET],
> + bd->rmap_fields[F_INT7_SET],
> + };
> +
> + /* Clear sub IRQs */
> + ret = regmap_read(bd->rmap, sub_status_reg[i], &sub_status);
> + if (ret) {
> + dev_err(bd->dev, "Failed to read IRQ sub-status\n");
> + goto err_umask;
> + }
Looking into it makes me thing that you perhaps need regmap IRQ chip?
Have you chance to look at drivers/mfd/intel_soc_pmic_bxtwc.c, for example?
> + ret = regmap_field_read(sub_mask_f[i], &sub_mask);
> + if (ret) {
> + dev_err(bd->dev, "Failed to read IRQ sub-mask\n");
> + goto err_umask;
> + }
> +
> + /* Ack active sub-statuses */
> + sub_status &= sub_mask;
> +
> + ret = regmap_write(bd->rmap, sub_status_reg[i], sub_status);
> + if (ret) {
> + dev_err(bd->dev, "Failed to ack sub-IRQ\n");
> + goto err_umask;
> + }
> + }
> +
> + ret = regmap_field_write(bd->rmap_fields[F_INT0_SET], mask);
> + if (ret)
> + /* May as well retry once */
> + goto err_umask;
> +
> + /* Read whole chip state */
> + ret = bd9995x_get_chip_state(bd, &state);
> + if (ret < 0) {
> + dev_err(bd->dev, "Failed to read chip state\n");
> + } else {
> + mutex_lock(&bd->lock);
> + bd->state = state;
> + mutex_unlock(&bd->lock);
> +
> + power_supply_changed(bd->charger);
> + }
> +
> + return IRQ_HANDLED;
> +
> +err_umask:
> + ret = regmap_field_write(bd->rmap_fields[F_INT0_SET], mask);
> + if (ret)
> + dev_err(bd->dev,
> + "Failed to un-mask F_INT0 - IRQ permanently disabled\n");
> +
> + return IRQ_NONE;
> +}
...
> +static int bd9995x_fw_probe(struct bd9995x_device *bd)
> +{
> + int ret;
> + struct power_supply_battery_info info;
> + u32 property;
> + int i;
> + int regval;
> + bool found;
> + struct bd9995x_init_data *init = &bd->init_data;
> + struct battery_init battery_inits[] = {
> + {
> + .name = "trickle-charging current",
> + .info_data = &info.tricklecharge_current_ua,
> + .range = &charging_current_ranges[0],
> + .ranges = 2,
> + .data = &init->itrich_set,
> + }, {
> + .name = "pre-charging current",
> + .info_data = &info.precharge_current_ua,
> + .range = &charging_current_ranges[0],
> + .ranges = 2,
> + .data = &init->iprech_set,
> + }, {
> + .name = "pre-to-trickle charge voltage threshold",
> + .info_data = &info.precharge_voltage_max_uv,
> + .range = &trickle_to_pre_threshold_ranges[0],
> + .ranges = 2,
> + .data = &init->vprechg_th_set,
> + }, {
> + .name = "charging termination current",
> + .info_data = &info.charge_term_current_ua,
> + .range = &charging_current_ranges[0],
> + .ranges = 2,
> + .data = &init->iterm_set,
> + }, {
> + .name = "charging re-start voltage",
> + .info_data = &info.charge_restart_voltage_uv,
> + .range = &charge_voltage_regulation_ranges[0],
> + .ranges = 2,
> + .data = &init->vrechg_set,
> + }, {
> + .name = "battery overvoltage limit",
> + .info_data = &info.overvoltage_limit_uv,
> + .range = &charge_voltage_regulation_ranges[0],
> + .ranges = 2,
> + .data = &init->vbatovp_set,
> + }, {
> + .name = "fast-charging max current",
> + .info_data = &info.constant_charge_current_max_ua,
> + .range = &fast_charge_current_ranges[0],
> + .ranges = 1,
> + .data = &init->ichg_set,
> + }, {
> + .name = "fast-charging voltage",
> + .info_data = &info.constant_charge_voltage_max_uv,
> + .range = &charge_voltage_regulation_ranges[0],
> + .ranges = 2,
> + .data = &init->vfastchg_reg_set1,
> + },
> + };
> + struct dt_init props[] = {
> + {
> + .prop = "rohm,vsys-regulation-microvolt",
> + .range = &vsys_voltage_regulation_ranges[0],
> + .ranges = 2,
> + .data = &init->vsysreg_set,
> + }, {
> + .prop = "rohm,vbus-input-current-limit-microamp",
> + .range = &input_current_limit_ranges[0],
> + .ranges = 1,
> + .data = &init->ibus_lim_set,
> + }, {
> + .prop = "rohm,vcc-input-current-limit-microamp",
> + .range = &input_current_limit_ranges[0],
> + .ranges = 1,
> + .data = &init->icc_lim_set,
> + },
> + };
> +
> + /*
> + * The power_supply_get_battery_info() does not support getting values
> + * from ACPI. Let's fix it if ACPI is required here.
> + */
Previously we discussed this and you told that you don't need ACPI support. Did
I get your wrong or something has been changed? If the latter, perhaps
converting power supply core to use device property API is not harder than what
you have done below.
> + ret = power_supply_get_battery_info(bd->charger, &info);
> + if (ret < 0)
> + return ret;
> +
> + for (i = 0; i < ARRAY_SIZE(battery_inits); i++) {
> + int val = *battery_inits[i].info_data;
> + const struct linear_range *range = battery_inits[i].range;
> + int ranges = battery_inits[i].ranges;
> +
> + if (val == -EINVAL)
> + continue;
> +
> + ret = linear_range_get_selector_low_array(range, ranges, val,
> + ®val, &found);
> + if (ret) {
> + dev_err(bd->dev, "Unsupported value for %s\n",
> + battery_inits[i].name);
> +
> + power_supply_put_battery_info(bd->charger, &info);
> + return -EINVAL;
> + }
> + if (!found) {
> + dev_warn(bd->dev,
> + "Unsupported value for %s - using smaller\n",
> + battery_inits[i].name);
> + }
> + *(battery_inits[i].data) = regval;
> + }
> +
> + power_supply_put_battery_info(bd->charger, &info);
> +
> + for (i = 0; i < ARRAY_SIZE(props); i++) {
> + ret = device_property_read_u32(bd->dev, props[i].prop,
> + &property);
> + if (ret < 0) {
> + dev_err(bd->dev, "failed to read %s", props[i].prop);
> +
> + return ret;
> + }
> +
> + ret = linear_range_get_selector_low_array(props[i].range,
> + props[i].ranges,
> + property, ®val,
> + &found);
> + if (ret) {
> + dev_err(bd->dev, "Unsupported value for '%s'\n",
> + props[i].prop);
> +
> + return -EINVAL;
> + }
> +
> + if (!found) {
> + dev_warn(bd->dev,
> + "Unsupported value for '%s' - using smaller\n",
> + props[i].prop);
> + }
> +
> + *(props[i].data) = regval;
> + }
> +
> + return 0;
> +}
--
With Best Regards,
Andy Shevchenko
